gh-111874: Call __set_name__ on objects that define the method inside a typing.NamedTuple class dictionary as part of the creation of that class by AlexWaygood · Pull Request #111876 · python/cpython (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation25 Commits17 Checks0 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
…d inside a typing.NamedTuple class dictionary as part of the creation of that class
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but I have few minor suggestions.
Thanks @serhiy-storchaka! I applied your suggestions; would you mind taking another look? I discovered another interesting thing in the process: __set_name__ was also not being called for the values of annotated fields (which become namedtuple members). I changed that so that __set_name__ is called on these field values also. It's consistent with what e.g. dataclasses does:
Python 3.13.0a1+ (heads/main:e5dfcc2b6e, Nov 15 2023, 17:23:51) [MSC v.1932 64 bit (AMD64)] on win32 Type "help", "copyright", "credits" or "license" for more information.
from dataclasses import dataclass class Vanilla: ... def set_name(self, owner, name): ... self.name = name ... @dataclass ... class Foo: ... v: Vanilla = Vanilla() ... f = Foo() f.v.name 'v'
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure that __set_name__ should be called for annotated fields.
I am not sure that
__set_name__should be called for annotated fields.
Yes, I could be persuaded either way there.
| setattr(nm_tpl, key, val) |
|---|
| try: |
| set_name = type(val).__set_name__ |
| except AttributeError: |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it worth adding a test for the case where this yields some other exception (e.g. due to a weird metaclass)?
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch. Seems like for normal classes, if any strange exception is raised when trying to lookup __set_name__, the interpreter just swallows it and moves on:
class Meta(type): ... def getattribute(self, attr): ... if attr == "set_name": ... raise BaseException("NO") ... class VeryAnnoying(metaclass=Meta): pass ... class Foo: ... attr = VeryAnnoying() ...
We should do the same here (though I don't want to swallow BaseExceptions, since that would catch KeyboardInterrupt, SystemExit, etc. -- I think I'll just use except Exception).
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like for normal classes, if any strange exception is raised when trying to lookup
__set_name__, the interpreter just swallows it and moves on:
It looks like a bug. I do not think it should be reproduced here. It is better to fix it for normal classes.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I opened #112453, and will revert the changes made here that reproduce the bug
Co-authored-by: Jelle Zijlstra jelle.zijlstra@gmail.com
…et_name__`"
This reverts commit 1909fd5.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
Few minor suggestions which you can ignore.
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request
Release 4.9.0 (December 9, 2023)
This feature release adds typing_extensions.ReadOnly, as specified
by PEP 705, and makes various other improvements, especially to
@typing_extensions.deprecated().
There are no changes since 4.9.0rc1.
Release 4.9.0rc1 (November 29, 2023)
- Add support for PEP 705, adding
typing_extensions.ReadOnly. Patch by Jelle Zijlstra. - All parameters on
NewType.__call__are now positional-only. This means that the signature oftyping_extensions.NewType.__call__now exactly matches the signature oftyping.NewType.__call__. Patch by Alex Waygood. - Fix bug with using
@deprecatedon a mixin class. Inheriting from a deprecated class now raises aDeprecationWarning. Patch by Jelle Zijlstra. @deprecatednow gives a better error message if you pass a non-strargument to themsgparameter. Patch by Alex Waygood.@deprecatedis now implemented as a class for better introspectability. Patch by Jelle Zijlstra.- Exclude
__match_args__fromProtocolmembers. Backport of python/cpython#110683 by Nikita Sobolev. - When creating a
typing_extensions.NamedTupleclass, ensure__set_name__is called on all objects that define__set_name__and exist in the values of theNamedTupleclass's class dictionary. Patch by Alex Waygood, backporting python/cpython#111876. - Improve the error message when trying to call
issubclass()against aProtocolthat has non-method members. Patch by Alex Waygood (backporting python/cpython#112344, by Randolph Scholz).
aisk pushed a commit to aisk/cpython that referenced this pull request
…d inside a typing.NamedTuple class dictionary as part of the creation of that class (python#111876)
Co-authored-by: Jelle Zijlstra jelle.zijlstra@gmail.com
Glyphack pushed a commit to Glyphack/cpython that referenced this pull request
…d inside a typing.NamedTuple class dictionary as part of the creation of that class (python#111876)
Co-authored-by: Jelle Zijlstra jelle.zijlstra@gmail.com