Repository navigation
bpo-43923: Allow NamedTuple multiple inheritance - #31779
juliusgeo wants to merge 10 commits into
Conversation
[@JelleZijlstra]
JelleZijlstra
left a comment
There was a problem hiding this comment.
This looks like it's still mostly a revert, which I don't think is the right approach since it ends up losing a bunch of past improvements. Try to make the minimal change necessary to make things work.
| def _make_nmtuple(name, types): | ||
| msg = "NamedTuple('Name', [(f0, t0), (f1, t1), ...]); each t must be a type" | ||
| types = [(n, _type_check(t, msg)) for n, t in types] | ||
| nm_tpl = collections.namedtuple(name, [n for n, t in types]) |
There was a problem hiding this comment.
collections.namedtuple now supports default= directly, so we should use that
| defaults=defaults, module=module) | ||
| nm_tpl.__annotations__ = nm_tpl.__new__.__annotations__ = types | ||
| def _make_nmtuple(name, types): | ||
| msg = "NamedTuple('Name', [(f0, t0), (f1, t1), ...]); each t must be a type" |
There was a problem hiding this comment.
You're missing the improved error message from the existing code
| default_names=', '.join(defaults_dict.keys()))) | ||
| nm_tpl.__new__.__annotations__ = dict(types) | ||
| nm_tpl.__new__.__defaults__ = tuple(defaults) | ||
| nm_tpl._field_defaults = defaults_dict |
There was a problem hiding this comment.
No need to restore this old attribute
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
|
Note there's an alternative PR from Serhiy: #31781. |
I still like Serhiy's more. In particular, I think it's best to keep |
https://bugs.python.org/issue43923
This is a fix-forward PR (partial revert of bpo-40185).
https://bugs.python.org/issue43923