-
-
Notifications
You must be signed in to change notification settings - Fork 31.6k
difflib.SequenceMatcher.find_longest_match default arguments #84574
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
Comments
The usage of difflib.SequenceMatcher.find_longest_match could be simplified for the most common use case (finding the longest match between the entirety of the two strings) by taking default args. At the moment you have to do: >>> from difflib import SequenceMatcher
>>> a, b = 'foo bar', 'foo baz'
>>> s = SequenceMatcher(a=a, b=b)
>>> s.find_longest_match(0, len(a), 0, len(b))
Match(a=0, b=0, size=6) but with default args the final line could be simplified to just: >>> s.find_longest_match()
Match(a=0, b=0, size=6) which seems to be much cleaned and more readable. I'd suggest updating the code so that the function signature becomes: find_longest_match(alo=None, ahi=None, blo=None, bhi=None) which is consistent with the current docstring of "Find longest matching block in a[alo:ahi] and b[blo:bhi]." as I think this would only be a minor code change, and if it is something that would be useful I'd be happy to have a go at a PR. |
Sounds good to me, Lewis - thanks! Note, though, that alo and blo should default to 0. |
Okay, that makes sense. I will raise a PR |
Adding a test for this and noticed I can add one more test case to get the method to full coverage. Can I add that to this PR or should I raise a separate one? |
I'm not clear on exactly what it is you're asking, but it's better to ask for forgiveness than permission ;-) That is, it's unlikely anyone will object to adding a test in a feature PR. |
Oh okay, well I was just saying I have added a test which is unrelated to the feature I have added, but it does test a different part of the same function. Anyway, I have raised a PR for this now (19742) and can separate it out if needed. |
All done. Thank you, Lewis! You're now an official Python contributor, and are entitled to all the fame, fortune, and power that follows. Use your new powers only for good :-) |
Thanks Tim. Cheers for your support with this :) |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: