[issue41388] IDLE fails to detect corresponding opening parenthesis
Lewis Ball added the comment: Okay that makes sense. Removing things from _synchre would slow down the matching slightly, although the matching still seems plenty fast enough when done inside an if clause at the moment, so I'm not sure how noticeable the removal of `else` would be. One thing could be to search for `else:` instead of just `else` in _synchre, as only the former indicates the start of a new statement (although I guess it would actually have to check for `else[\s\\]*:` or something). Like you say though, if there are other more pressing issues with the matching then maybe it is worth fixing them all at the same time. Happy to help with that if needed. -- ___ Python tracker <https://bugs.python.org/issue41388> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue41388] IDLE fails to detect corresponding opening parenthesis
Lewis Ball added the comment: Okay, on further examination `find_good_parse_start` first looks for a line ending with ':\n', and then looks to match _synchre on that line. If it can't find any line ending in ':\n' then it will just look for the first occurrence of something in _synchre to truncate on. If it can't find anything at that point, it just doesn't truncate and the whole code is checked for a bracket match. So the reason that ``` else ( else) ``` works is because there is no `:` after the first else, and adding `:` will cause the matching to fail. This seems reasonable, and I was probably too quick to say that the function goes back further than it needs to when looking to truncate. It seems like then that `else` being removed from _synchre will fix this, and mean that the only time brackets aren't matched are when invalid code is typed, something like: ``` ( def) ``` I can put in a PR for this tomorrow (probably removing `yield` at the same time) if this sounds like the right fix. P.S. Here is an example of the similar `yield` error that Terry alluded to: ``` def f(): for i in range(10): ( yield x) ``` -- ___ Python tracker <https://bugs.python.org/issue41388> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue41388] IDLE fails to detect corresponding opening parenthesis
Lewis Ball added the comment: So it looks like `pyparse.Parser.find_good_parse_start` is responsible for truncating the code to only look for the matching bracket in the current statement, which uses _synchre. Testing it out, it sometimes will go to the most recent match of _synchre, but will sometimes go back to an even earlier match in the code, which is why something like ``` else ( else) ``` manages to match without an issue. The `find_good_parse_start` will truncate at the first `else` in this case, instead at the second one. Removing `else` from the _synchre regex did solve this problem for me though, as `find_good_parse_start` will then try to truncate even earlier when looking for the start of the statement, which will be before the opener. Although, I haven't checked what else would be affected by this change. I am not sure why this worked for me and did not work for you Terry. Also, even with this fix it seems like 'find_good_parse_start` goes back further than it needs to. Fixing that may not change much, but would offer a slight performance increase. -- ___ Python tracker <https://bugs.python.org/issue41388> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue41388] IDLE fails to detect corresponding opening parenthesis
Lewis Ball added the comment: A minimal example of the same issue seems to be: ``` (1 if x else 0) ``` In this case the parentheses matching also fails. It only ever seems to fail when `else` is the first word on a newline inside brackets, and adding the line break elsewhere in the above statement will not cause the matching to fail. I imagine it stops looking for the matching parentheses when it gets to the `else` statement, as an `else` when part of an `if/else` cannot be inside parentheses. Obviously the exception to this is the ternary operator. Bizarrely, it only seems to fail the first time it encounters this issue. Any subsequent code with the same pattern seems to be matched fine. So in the following example, the second brackets get matched without an issue: ``` (1 if x else 0) (1 if x else 0) ``` Hopefully someone more familiar with the source code can shed some light on this! -- nosy: +Lewis Ball ___ Python tracker <https://bugs.python.org/issue41388> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue41387] Escape needed in the email documentation example
Lewis Ball added the comment: Okay no worries, glad it is all sorted now :) -- ___ Python tracker <https://bugs.python.org/issue41387> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue41387] Escape needed in the email documentation example
Lewis Ball added the comment: So I have just tried it out and the example seems to work fine for me as it is (tested using 3.8.2). Note that escaping the " in this case makes no difference to the string: ``` >>> """""" == """""" True ``` -- nosy: +Lewis Ball ___ Python tracker <https://bugs.python.org/issue41387> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue40474] Code coverage report not entirely accurate
Lewis Ball added the comment: I don't have triage permissions so I don't think I can request a review. Although @aeros has requested one. -- ___ Python tracker <https://bugs.python.org/issue40474> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue40539] Docs - difflib.SequenceMatcher quick_ratio and real_quick_ratio improved docs
Change by Lewis Ball : -- keywords: +patch pull_requests: +19287 stage: -> patch review pull_request: https://github.com/python/cpython/pull/19971 ___ Python tracker <https://bugs.python.org/issue40539> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue40539] Docs - difflib.SequenceMatcher quick_ratio and real_quick_ratio improved docs
New submission from Lewis Ball : Currently the docs for `difflib.SequenceMatcher.quick_ratio()` just says 'Return an upper bound on ratio() relatively quickly', which doesn't give much of an idea about how that upper bound is calculated. `real_quick_ratio` has similarly brief documentation. I'll raise a PR shortly to add a more verbose description to each of these ratios, so that it is clear when each should be used. My current suggestions would be: quick_ratio Return an upper bound on ratio() relatively quickly. This is the highest possible ratio() given these letters, regardless of their order. real_quick_ratio Return an upper bound on ratio() very quickly. This is the highest possible ratio() given the lengths of a and b, regardless of their letters. i.e. 2*(min(len(a), len(b))/(len(a) + len(b)) -- assignee: docs@python components: Documentation messages: 368305 nosy: Lewis Ball, docs@python priority: normal severity: normal status: open title: Docs - difflib.SequenceMatcher quick_ratio and real_quick_ratio improved docs type: enhancement versions: Python 3.9 ___ Python tracker <https://bugs.python.org/issue40539> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue40474] Code coverage report not entirely accurate
Lewis Ball added the comment: I've updated the travis.yml now to fix this issue and global statements are now showing as covered in the coverage report. This means an extra 4k lines show as covered which weren't previously showing. See the report for the PR here (https://codecov.io/gh/python/cpython/tree/64d521b5d34c25b83d0472608d1eab3a6334bf59/Lib). -- ___ Python tracker <https://bugs.python.org/issue40474> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue40474] Code coverage report not entirely accurate
Change by Lewis Ball : -- keywords: +patch pull_requests: +19168 stage: -> patch review pull_request: https://github.com/python/cpython/pull/19851 ___ Python tracker <https://bugs.python.org/issue40474> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue40474] Code coverage report not entirely accurate
New submission from Lewis Ball : The current reported code coverage stats on codecov.io miss some global statements of modules imported before coveragepy begins. This is documented in the dev guide, and the suggested fix is here: https://devguide.python.org/coverage/#coverage-results-for-modules-imported-early-on but that recommended fix doesn't seem to be included in the CI. Using the recommended 'hack', the coverage report shows an extra 4k lines are coverage in Lib, increasing coverage percentage by around 3%. Having a more accurate view of the coverage makes it much easier to see which parts of the code need help with tests, without having to go into every individual report to see if global statements are misreported. I'll raise a PR for this shortly -- components: Tests messages: 367904 nosy: Lewis Ball priority: normal severity: normal status: open title: Code coverage report not entirely accurate type: enhancement versions: Python 3.9 ___ Python tracker <https://bugs.python.org/issue40474> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue40394] difflib.SequenceMatcher.find_longest_match default arguments
Lewis Ball added the comment: Thanks Tim. Cheers for your support with this :) -- ___ Python tracker <https://bugs.python.org/issue40394> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue40394] difflib.SequenceMatcher.find_longest_match default arguments
Lewis Ball added the comment: 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. -- ___ Python tracker <https://bugs.python.org/issue40394> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue40394] difflib.SequenceMatcher.find_longest_match default arguments
Change by Lewis Ball : -- keywords: +patch pull_requests: +19064 stage: needs patch -> patch review pull_request: https://github.com/python/cpython/pull/19742 ___ Python tracker <https://bugs.python.org/issue40394> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue40394] difflib.SequenceMatcher.find_longest_match default arguments
Lewis Ball added the comment: 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? -- ___ Python tracker <https://bugs.python.org/issue40394> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue40394] difflib.SequenceMatcher.find_longest_match default arguments
Lewis Ball added the comment: Okay, that makes sense. I will raise a PR -- ___ Python tracker <https://bugs.python.org/issue40394> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue40394] difflib.SequenceMatcher.find_longest_match default arguments
New submission from Lewis Ball : 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. -- components: Library (Lib) messages: 367306 nosy: Lewis Ball priority: normal severity: normal status: open title: difflib.SequenceMatcher.find_longest_match default arguments type: enhancement ___ Python tracker <https://bugs.python.org/issue40394> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
