FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

[WIP] gh-132166: Add check in find_longest_match for checking if sequences are identical by jurgenwigg · Pull Request #132167 · python/cpython · GitHub

  1. python
  2. cpython

Repository navigation

[WIP] gh-132166: Add check in find_longest_match for checking if sequences are identical - #132167

Closed
jurgenwigg wants to merge 4 commits into
python:mainfrom
jurgenwigg:gh-132166-check-sequences-if-they-are-identical
ConversationCommits (4)ChecksFiles changed
Closed

[WIP] gh-132166: Add check in find_longest_match for checking if sequences are identical#132167
jurgenwigg wants to merge 4 commits into
python:mainfrom
jurgenwigg:gh-132166-check-sequences-if-they-are-identical

Conversation

jurgenwigg commented Apr 6, 2025 •
edited by bedevere-app Bot
Loading

Copy link
Copy Markdown

As mentioned in #132166 added checking if both sequences are identical at the beginning of the method find_longest_match in SequenceMatcher class.

bedevere-app Bot commented Apr 6, 2025

Copy link
Copy Markdown

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

bedevere-app Bot mentioned this pull request Apr 6, 2025
Open
sobolevn reviewed Apr 6, 2025
View reviewed changes

sobolevn left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Please, see my question in #132166 (comment)

brianschubert reviewed Apr 6, 2025
View reviewed changes
Comment thread Lib/difflib.py Outdated
picnixz reviewed Apr 6, 2025
View reviewed changes
Comment thread Misc/NEWS.d/next/Core_and_Builtins/2025-04-06-17-31-44.gh-issue-132166.tbo9yR.rst
Comment on lines +4 to +5
It appears to fix issue when comparing two slightly different strings ends up
with waiting forever for the result.

picnixz Apr 6, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality
Suggested change
It appears to fix issue when comparing two slightly different strings ends up
with waiting forever for the result.
It appears to fix issue when comparing two slightly different strings ends up
with waiting forever for the result.

We should be more confident here. Does it fix the issue or not? if so, let's say it. Otherwise, let's not give tentative conclusions.

Comment thread Misc/NEWS.d/next/Core_and_Builtins/2025-04-06-17-31-44.gh-issue-132166.tbo9yR.rst
Comment on lines +1 to +3
Add checking if sequences ``a[alo:ahi]`` and ``b[blo:bhi]`` are the same on the
beginning of the method find_longest_match in SequenceMatcher. For identical
sequences there is no reason to run whole logic when simple check can be done.

picnixz Apr 6, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

This NEWS should be summarized. I suggest something like (assumiong that the method is documented, if not use a plain

Add a fast path in :meth:`SequenceMatcher.find_longest_match
<difflib.SequenceMatcher.find_longest_match>` to handle the
case when ``a[alo:ahi] == b[blo:bhi]``.

Comment thread Lib/difflib.py

# Check if both sequences are the same before executing rest of the
# method.
if a[alo:ahi] == b[blo:bhi]:

picnixz Apr 6, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Is this covered in tests? if not, please add a test, otherwise no need to add a large one.

jurgenwigg changed the title gh-132166: Add check in find_longest_match for checking if sequences are identical [WIP] gh-132166: Add check in find_longest_match for checking if sequences are identical Apr 7, 2025

python-cla-bot Bot commented Apr 18, 2025

Copy link
Copy Markdown

The following commit authors need to sign the Contributor License Agreement:

dg-pb commented Feb 12, 2026

Copy link
Copy Markdown
Contributor

I am working on improvements here.

Please hold out on this.

dg-pb commented Feb 12, 2026

Copy link
Copy Markdown
Contributor

Also, I think this breaks backwards compatibility when both autojunk=True and isjunk is not None.

github-actions Bot commented Apr 23, 2026

Copy link
Copy Markdown

This PR is stale because it has been open for 30 days with no activity.

github-actions Bot added the stale Stale PR or inactive for long period of time. label Apr 23, 2026

StanFromIreland commented Oct 23, 2026

Copy link
Copy Markdown
Member

Closing since the CLA is unsigned, so we cannot move this forward.

dg-pb commented Oct 23, 2026

Copy link
Copy Markdown
Contributor

If someone is interested in accelerating improvements here:

  1. I am waiting for review of gh-145209: Composability of difflib #145210
  2. Once that is merged (or not) I will issue PR with algorithmic improvement for current SequenceMatcher described in https://discuss.python.org/t/improvement-on-current-difflib-sequencematcher-algorithm/106221

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

sobolevn sobolevn left review comments
picnixz picnixz left review comments
brianschubert brianschubert left review comments

Assignees

No one assigned

Labels

awaiting review stale Stale PR or inactive for long period of time.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

Back | FazBrowse Home | New Git URL