This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: difflib.SequenceMatcher.find_longest_match default arguments
Type: enhancement Stage: resolved
Components: Library (Lib) Versions: Python 3.9
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: tim.peters Nosy List: Lewis Ball, tim.peters
Priority: normal Keywords: patch

Created on 2020-04-26 14:15 by Lewis Ball, last changed 2022-04-11 14:59 by admin. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 19742 merged Lewis Ball, 2020-04-27 21:52
Messages (9)
msg367306 - (view) Author: Lewis Ball (Lewis Ball) * Date: 2020-04-26 14:15
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 `a[None:None]` is the whole of `a`.

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.
msg367380 - (view) Author: Tim Peters (tim.peters) * (Python committer) Date: 2020-04-27 04:00
Sounds good to me, Lewis - thanks!  Note, though, that alo and blo should default to 0.  `None` is best reserved for cases where the default value needs to be computed at runtime.  But alo == blo == 0 apply to all possible instances.
msg367456 - (view) Author: Lewis Ball (Lewis Ball) * Date: 2020-04-27 20:34
Okay, that makes sense. I will raise a PR
msg367459 - (view) Author: Lewis Ball (Lewis Ball) * Date: 2020-04-27 21:18
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?
msg367461 - (view) Author: Tim Peters (tim.peters) * (Python committer) Date: 2020-04-27 21:27
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.
msg367464 - (view) Author: Lewis Ball (Lewis Ball) * Date: 2020-04-27 21:55
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.
msg367731 - (view) Author: Tim Peters (tim.peters) * (Python committer) Date: 2020-04-30 03:42
New changeset 3209cbd99b6d65aa18b3beb124fac9c792b8993d by lrjball in branch 'master':
bpo-40394 - difflib.SequenceMatched.find_longest_match default args (GH-19742)
https://github.com/python/cpython/commit/3209cbd99b6d65aa18b3beb124fac9c792b8993d
msg367732 - (view) Author: Tim Peters (tim.peters) * (Python committer) Date: 2020-04-30 03:45
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 :-)
msg367902 - (view) Author: Lewis Ball (Lewis Ball) * Date: 2020-05-02 00:12
Thanks Tim. Cheers for your support with this :)
History
Date User Action Args
2022-04-11 14:59:29adminsetgithub: 84574
2020-05-02 00:12:06Lewis Ballsetmessages: + msg367902
2020-04-30 03:45:43tim.peterssetstatus: open -> closed
resolution: fixed
messages: + msg367732

stage: patch review -> resolved
2020-04-30 03:42:51tim.peterssetmessages: + msg367731
2020-04-27 21:55:19Lewis Ballsetmessages: + msg367464
2020-04-27 21:52:36Lewis Ballsetkeywords: + patch
stage: needs patch -> patch review
pull_requests: + pull_request19064
2020-04-27 21:27:46tim.peterssetstage: needs patch
messages: + msg367461
versions: + Python 3.9
2020-04-27 21:18:55Lewis Ballsetmessages: + msg367459
2020-04-27 20:34:48Lewis Ballsetmessages: + msg367456
2020-04-27 04:00:42tim.peterssetmessages: + msg367380
2020-04-26 23:59:47rhettingersetassignee: tim.peters
2020-04-26 14:24:33xtreaksetnosy: + tim.peters
2020-04-26 14:15:58Lewis Ballcreate