-
Notifications
You must be signed in to change notification settings - Fork 258
Allow keyword syntax for NamedTuple #321
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
Conversation
@@ -1985,20 +1987,29 @@ class Employee(NamedTuple): | |||
The resulting class has one extra attribute: _field_types, | |||
giving a dict mapping field names to types. (The field names | |||
are in the _fields attribute, which is part of the namedtuple | |||
API.) Backward-compatible usage:: | |||
API.) Alternative equivalent keyword syntax is also accepted:: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you change the earlier part of the docstring (line 1975) to emphasize that the both the PEP 526 syntax and the keyword args syntax are only supported in Python 3.6+? (I can't add github comments that far back.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gvanrossum Note that this docstring will be seen only by people who use Python 3.6+, this docstring is inside a big if sys.version_info[:2] >= (3, 6):
above. Anyway, I updated the docstring to cover this and next comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gvanrossum OK, now there is no big if sys.version_info[:2] >= (3, 6):
, there is the same code and docstring for all versions.
def __new__(self, typename, fields): | ||
def __new__(self, typename, fields=None, **kwargs): | ||
if fields is None: | ||
fields = kwargs.items() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be disallowed for Python <= 3.5. (Remember that this code will be in the "typing" package which should work for Python 3.2 and up, and it will be copied verbatim into the 3.5 branch as well.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gvanrossum This code is also inside if sys.version_info[:2] >= (3, 6):
I mentioned above. So that the else:
branch is used for other versions, and both keyword and class syntax fail with a TypeError
.
I will add a new commit now, where I will "unify" the code, this will give better error messages.
|
||
Employee = NamedTuple('Employee', name=str, id=int) | ||
|
||
Backward-compatible usage:: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So "backward-compatible" sounds a bit pejorative. I think this should just be referred to as the way to do it for Python versions <= 3.5.
FWIW I'm fine with having this in 3.6 without supporting it in mypy. Once 3.6 is released we can't change typing.py easily (not until 3.6.1 anyways) but we can add support to mypy at any time. |
@gvanrossum Thank you for review! I have implemented your comments.
and
Please take a look! |
@gvanrossum Does this look OK now? |
OK. Another leap of faith. :-) Did you have other changes to typing.py? Or can I do the final merge into CPython and mypy now? |
@gvanrossum Thanks! :-)
There is #322 , but note that mypy support for |
Fixes #302
@gvanrossum Here is a simple implementation for keyword syntax that you proposed:
Note that this requires 3.6+ since order of kwargs should be preserved.