Repository navigation
[WIP] gh-132166: Add check in find_longest_match for checking if sequences are identical - #132167
jurgenwigg wants to merge 4 commits into
Conversation
|
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 |
sobolevn
left a comment
There was a problem hiding this comment.
Please, see my question in #132166 (comment)
| It appears to fix issue when comparing two slightly different strings ends up | ||
| with waiting forever for the result. |
There was a problem hiding this comment.
| 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.
| 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. |
There was a problem hiding this comment.
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]``.|
|
||
| # Check if both sequences are the same before executing rest of the | ||
| # method. | ||
| if a[alo:ahi] == b[blo:bhi]: |
There was a problem hiding this comment.
Is this covered in tests? if not, please add a test, otherwise no need to add a large one.
|
The following commit authors need to sign the Contributor License Agreement: |
|
I am working on improvements here. Please hold out on this. |
|
Also, I think this breaks backwards compatibility when both |
|
This PR is stale because it has been open for 30 days with no activity. |
|
Closing since the CLA is unsigned, so we cannot move this forward. |
|
If someone is interested in accelerating improvements here:
|
As mentioned in #132166 added checking if both sequences are identical at the beginning of the method
find_longest_matchinSequenceMatcherclass.SequenceMatcher.find_longest_match#132166