bpo-33064: lib2to3: support trailing comma after **kwargs #6096
Conversation
I also made the comments in line with the builtin Grammar/Grammar. PEP 306 was withdrawn, Kees Blom's railroad program has been lost to the sands of time for at least 16 years now (I found a python-dev post from people looking for it).
There was a problem hiding this comment.
No review comments on the grammar changes. Perhaps @benjaminp has an interest in them? Otherwise, the NEWS entry cruft should be removed.
|
|
||
| # Write your Misc/NEWS entry below. It should be a simple ReST paragraph. # | ||
| Don't start with "- Issue #<n>: " or "- bpo-<n>: " or that sort of stuff. | ||
| ########################################################################### |
There was a problem hiding this comment.
Some extraneous blurb stuff to remove here.
|
When you're done making the requested changes, leave the comment: |
|
Grammar change seems okay to me. |
I actually looked at the changes before approving the PR :) |
|
@1st1 I was sure you did. I just meant that I didn't! |
|
I have made the requested changes; please review again. |
|
Thanks for making the requested changes! @ned-deily, @1st1: please review the changes made to this pull request. |
|
Updated NEWS again, suspicious checker failed on this because there were no backticks around **kwargs. |
|
Any tests? |
|
Good call, Serhiy, I added some :) |
| @@ -305,6 +303,17 @@ def test_8(self): | |||
| *g:6, h:7, i=8, j:9=10, **k:11) -> 12: pass""" | |||
| self.validate(s) | |||
|
|
|||
| def test_9(self): | |||
There was a problem hiding this comment.
Would be nice to add tests for signatures like:
(a,)
(*args,)
(a=1,)
and few combinations if they have a separate path in the grammar.
There was a problem hiding this comment.
LOL, that's one subtle way to signal "your patch doesn't handle the vararg case" ;-) Alright, I added *args support, too, and added the relevant tests.
|
@ambv: Please replace |
|
Thanks @ambv for the PR 🌮🎉.. I'm working now to backport this PR to: 3.6, 3.7. |
…ythonGH-6096) New tests also added. I also made the comments in line with the builtin Grammar/Grammar. PEP 306 was withdrawn, Kees Blom's railroad program has been lost to the sands of time for at least 16 years now (I found a python-dev post from people looking for it). (cherry picked from commit b51f5de) Co-authored-by: Łukasz Langa <lukasz@langa.pl>
|
GH-6097 is a backport of this pull request to the 3.7 branch. |
…ythonGH-6096) New tests also added. I also made the comments in line with the builtin Grammar/Grammar. PEP 306 was withdrawn, Kees Blom's railroad program has been lost to the sands of time for at least 16 years now (I found a python-dev post from people looking for it). (cherry picked from commit b51f5de) Co-authored-by: Łukasz Langa <lukasz@langa.pl>
|
GH-6098 is a backport of this pull request to the 3.6 branch. |
There was a problem hiding this comment.
I was unsure. The grammar rules are hard to read, tests allow to confirm suspicions.
I have doubts about some combinations. I would add tests for
(a, b)
(a, *b)
(a, b=1)
(a, **b)
(*a, b=1)
(*a, **b)
(*, b=1)
(a=1, b=2)
(a=1, **b)
with and without a trailing comma.
| @@ -38,13 +19,13 @@ async_funcdef: 'async' funcdef | |||
| funcdef: 'def' NAME parameters ['->' test] ':' suite | |||
| parameters: '(' [typedargslist] ')' | |||
| typedargslist: ((tfpdef ['=' test] ',')* | |||
| ('*' [tname] (',' tname ['=' test])* [',' '**' tname] | '**' tname) | |||
| ('*' [tname] (',' tname ['=' test])* [',' ['**' tname [',']]] | '**' tname [',']) | |||
| | tfpdef ['=' test] (',' tfpdef ['=' test])* [',']) | |||
There was a problem hiding this comment.
Could it be just tfpdef ['=' test] [',']? Of course may be there are reasons of writing the rule as it is written due to the parser limitations.
There was a problem hiding this comment.
Maybe but I want to keep this as close to Grammar/Grammar and this is how it's voiced there.
…H-6096) (#6097) New tests also added. I also made the comments in line with the builtin Grammar/Grammar. PEP 306 was withdrawn, Kees Blom's railroad program has been lost to the sands of time for at least 16 years now (I found a python-dev post from people looking for it). (cherry picked from commit b51f5de) Co-authored-by: Łukasz Langa <lukasz@langa.pl>
…H-6096) (#6098) New tests also added. I also made the comments in line with the builtin Grammar/Grammar. PEP 306 was withdrawn, Kees Blom's railroad program has been lost to the sands of time for at least 16 years now (I found a python-dev post from people looking for it). (cherry picked from commit b51f5de) Co-authored-by: Łukasz Langa <lukasz@langa.pl>
|
@serhiy-storchaka, I merged this before I read your comment on the additional tests you'd like to see. I added the combinations you described in a separate pull request (see #6101). Thanks for your thorough review! |
…ython#6096) New tests also added. I also made the comments in line with the builtin Grammar/Grammar. PEP 306 was withdrawn, Kees Blom's railroad program has been lost to the sands of time for at least 16 years now (I found a python-dev post from people looking for it).
Title says it like it is.
I also made the comments in line with the builtin Grammar/Grammar. PEP 306 was
withdrawn, Kees Blom's railroad program has been lost to the sands of time for
at least 16 years now (I found a python-dev post from people looking for it).
It would be actually nice to make the rest of the grammar uniform with Grammar/Grammar whenever possible to be able to diff it easier for omissions. Of course, some differences will always be there to support Python 2 syntax, but a lot could be improved. I'll leave that for a separate commit though.
https://bugs.python.org/issue33064