bpo-43923: Allow NamedTuple multiple inheritance #31779
Conversation
juliusgeo
requested review from
gvanrossum,
Fidget-Spinner and
JelleZijlstra
as code owners
Lib/typing.py
Outdated
| 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
Lib/typing.py
Outdated
| 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
Lib/typing.py
Outdated
| 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 |
|
We merged #92027 to allow generic namedtuples. I don't think we want more general multiple inheritance. Thanks for your PR though! |
https://bugs.python.org/issue43923
This is a fix-forward PR (partial revert of bpo-40185).
https://bugs.python.org/issue43923