Skip to content

gh-130167: Minor textwrap.dedent() optimization #131925

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Marius-Juston wants to merge 2 commits into from
Closed

gh-130167: Minor textwrap.dedent() optimization #131925

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2b3f500
minor optimizations for simulataneously comptuing min + max + normali…
Marius-Juston Mar 31, 2025
d30196e
replacing if with elseif
Marius-Juston Mar 31, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 18 additions & 5 deletions Lib/textwrap.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -432,15 +432,28 @@ def dedent(text):
lines = text.split('\n')

# Get length of leading whitespace, inspired by ``os.path.commonprefix()``.
non_blank_lines = [l for l in lines if l and not l.isspace()]
l1 = min(non_blank_lines, default='')
l2 = max(non_blank_lines, default='')
margin = 0
l1 = None
Copy link
Copy Markdown
Contributor

@eendebakpt eendebakpt Mar 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
l1 = None
l1 = ''

You can then skip the is None check in the loop. Similar for l2 (but you have to pick the default with care)

l2 = None
for i, line in enumerate(lines):
# Compute min + max concurrently + normalize others
if line and not line.isspace():
if l1 is None or line < l1:
l1 = line
if l2 is None or line > l2:
Copy link
Copy Markdown
Contributor

@eendebakpt eendebakpt Mar 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once you skip the if None check, this if can be an elseif

l2 = line
else:
lines[i] = ''

if l1 is None:
l1 = ''

Comment on lines +435 to +453
Copy link
Copy Markdown
Contributor

@eendebakpt eendebakpt Mar 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
val = False
for i, line in enumerate(lines):
# Compute min + max concurrently + normalize others
if line and not line.isspace():
if val:
if line < l1:
l1 = line
elif line > l2:
l2 = line
else:
val = True
l1 = l2 = line
else:
lines[i] = ''
if not val or not l1:
return '\n'.join(lines)
l1 = 'z'
l2 = ''
for i, line in enumerate(lines):
# Compute min + max concurrently + normalize others
if line and not line.isspace():
if line < l1:
l1 = line
if line > l2:
l2 = line
else:
lines[i] = ''
if not l1:
return '\n'.join(lines)
margin = 0
for margin, c in enumerate(l2):
if c != l1[margin] or c not in ' \t':
break

This saves chechking the val. Performance gain is only a bit though.

I agree with the other reviewers: unless we find out a way to make this much faster, the small gains are not worth the change.

@Marius-Juston Thanks for putting in the work to get all the performance numbers!

for margin, c in enumerate(l1):
if c != l2[margin] or c not in ' \t':
break
else:
return '\n'.join(lines)

return '\n'.join([l[margin:] if not l.isspace() else '' for l in lines])
return '\n'.join([line[margin:] for line in lines])


def indent(text, prefix, predicate=None):
Expand Down
Loading