-
-
Notifications
You must be signed in to change notification settings - Fork 31.6k
[WIP] gh-132166: Add check in find_longest_match for checking if sequences are identical #132167
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,5 @@ | ||||||||||
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. | ||||||||||
Comment on lines
+1
to
+3
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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]``. |
||||||||||
It appears to fix issue when comparing two slightly different strings ends up | ||||||||||
with waiting forever for the result. | ||||||||||
Comment on lines
+4
to
+5
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this covered in tests? if not, please add a test, otherwise no need to add a large one.