From e3b703d2a8c715f9ba0aef3615b9d4c9506124d7 Mon Sep 17 00:00:00 2001 From: AlexWaygood Date: Thu, 9 Nov 2023 11:08:24 +0000 Subject: [PATCH 01/15] 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 --- Lib/test/test_typing.py | 49 +++++++++++++++++++ Lib/typing.py | 13 ++++- ...-11-09-11-07-34.gh-issue-111874.dzYc3j.rst | 4 ++ 3 files changed, 64 insertions(+), 2 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2023-11-09-11-07-34.gh-issue-111874.dzYc3j.rst diff --git a/Lib/test/test_typing.py b/Lib/test/test_typing.py index 1e812e1d525a81..709601ff162639 100644 --- a/Lib/test/test_typing.py +++ b/Lib/test/test_typing.py @@ -7519,6 +7519,55 @@ class GenericNamedTuple(NamedTuple, Generic[T]): self.assertEqual(CallNamedTuple.__orig_bases__, (NamedTuple,)) + def test_setname_called_on_non_members(self): + class Vanilla: + def __set_name__(self, owner, name): + self.name = name + + class Foo(NamedTuple): + attr = Vanilla() + + foo = Foo() + self.assertEqual(len(foo), 0) + self.assertNotIn('attr', Foo._fields) + self.assertIsInstance(foo.attr, Vanilla) + self.assertEqual(foo.attr.name, "attr") + + def test_setname_raises_the_same_as_on_other_classes(self): + class CustomException(Exception): pass + + class Annoying: + def __set_name__(self, owner, name): + raise CustomException("Cannot do that!") + + with self.assertRaisesRegex(CustomException, "Cannot do that!") as cm: + class NormalClass: + attr = Annoying() + normal_exception = cm.exception + + with self.assertRaisesRegex(CustomException, "Cannot do that!") as cm: + class NamedTupleClass: + attr = Annoying() + namedtuple_exception = cm.exception + + self.assertIs(type(namedtuple_exception), CustomException) + self.assertIs(type(namedtuple_exception), type(normal_exception)) + + self.assertEqual(len(namedtuple_exception.__notes__), 1) + self.assertEqual( + len(namedtuple_exception.__notes__), len(normal_exception.__notes__) + ) + + expected_note = ( + "Error calling __set_name__ on 'Annoying' instance " + "'attr' in 'NamedTupleClass'" + ) + self.assertEqual(namedtuple_exception.__notes__[0], expected_note) + self.assertEqual( + namedtuple_exception.__notes__[0], + normal_exception.__notes__[0].replace("NormalClass", "NamedTupleClass") + ) + class TypedDictTests(BaseTestCase): def test_basics_functional_syntax(self): diff --git a/Lib/typing.py b/Lib/typing.py index 14845b36028ca1..b08544db4994d7 100644 --- a/Lib/typing.py +++ b/Lib/typing.py @@ -2726,11 +2726,20 @@ def __new__(cls, typename, bases, ns): class_getitem = _generic_class_getitem nm_tpl.__class_getitem__ = classmethod(class_getitem) # update from user namespace without overriding special namedtuple attributes - for key in ns: + for key, val in ns.items(): if key in _prohibited: raise AttributeError("Cannot overwrite NamedTuple attribute " + key) elif key not in _special and key not in nm_tpl._fields: - setattr(nm_tpl, key, ns[key]) + setattr(nm_tpl, key, val) + if hasattr(type(val), "__set_name__"): + try: + type(val).__set_name__(val, nm_tpl, key) + except Exception as e: + e.add_note( + f"Error calling __set_name__ on {type(val).__name__!r} " + f"instance {key!r} in {typename!r}" + ) + raise if Generic in bases: nm_tpl.__init_subclass__() return nm_tpl diff --git a/Misc/NEWS.d/next/Library/2023-11-09-11-07-34.gh-issue-111874.dzYc3j.rst b/Misc/NEWS.d/next/Library/2023-11-09-11-07-34.gh-issue-111874.dzYc3j.rst new file mode 100644 index 00000000000000..4d9f3d7bc22d20 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2023-11-09-11-07-34.gh-issue-111874.dzYc3j.rst @@ -0,0 +1,4 @@ +When creating a :class:`typing.NamedTuple` class, ensure +:func:`__set_name__` is called on all objects that define ``__set_name__`` +and exist in the values of the ``NamedTuple`` class's class dictionary. +Patch by Alex Waygood. From f05331028615aa37663f0d888a26b884c1c57eee Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Thu, 9 Nov 2023 11:12:37 +0000 Subject: [PATCH 02/15] fixup news --- .../Library/2023-11-09-11-07-34.gh-issue-111874.dzYc3j.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Misc/NEWS.d/next/Library/2023-11-09-11-07-34.gh-issue-111874.dzYc3j.rst b/Misc/NEWS.d/next/Library/2023-11-09-11-07-34.gh-issue-111874.dzYc3j.rst index 4d9f3d7bc22d20..50408202a7a5a1 100644 --- a/Misc/NEWS.d/next/Library/2023-11-09-11-07-34.gh-issue-111874.dzYc3j.rst +++ b/Misc/NEWS.d/next/Library/2023-11-09-11-07-34.gh-issue-111874.dzYc3j.rst @@ -1,4 +1,4 @@ When creating a :class:`typing.NamedTuple` class, ensure -:func:`__set_name__` is called on all objects that define ``__set_name__`` -and exist in the values of the ``NamedTuple`` class's class dictionary. -Patch by Alex Waygood. +:func:`~object.__set_name__` is called on all objects that define +``__set_name__`` and exist in the values of the ``NamedTuple`` class's class +dictionary. Patch by Alex Waygood. From 7ccff63801ce33522b83bd2e2b89a0ef1500aa0e Mon Sep 17 00:00:00 2001 From: AlexWaygood Date: Thu, 9 Nov 2023 11:34:31 +0000 Subject: [PATCH 03/15] Also add a test for TypedDict classes --- Lib/test/test_typing.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/Lib/test/test_typing.py b/Lib/test/test_typing.py index 709601ff162639..012733eb60d688 100644 --- a/Lib/test/test_typing.py +++ b/Lib/test/test_typing.py @@ -8101,6 +8101,23 @@ class T4(TypedDict, Generic[S]): pass self.assertEqual(klass.__optional_keys__, set()) self.assertIsInstance(klass(), dict) + def test_setname_called_on_things_in_class_namespace(self): + class CustomException(Exception): pass + + class Annoying: + def __set_name__(self, owner, name): + raise CustomException("Cannot do that!") + + with self.assertRaisesRegex(CustomException, "Cannot do that!") as cm: + class Foo(TypedDict): + attr = Annoying() + + expected_note = ( + "Error calling __set_name__ on 'Annoying' instance " + "'attr' in 'Foo'" + ) + self.assertIn(expected_note, cm.exception.__notes__) + class RequiredTests(BaseTestCase): From e4e968a6988b3274c3bac468bf202b3f22a1b259 Mon Sep 17 00:00:00 2001 From: AlexWaygood Date: Thu, 9 Nov 2023 13:48:51 +0000 Subject: [PATCH 04/15] Revert "Also add a test for TypedDict classes" This reverts commit 7ccff63801ce33522b83bd2e2b89a0ef1500aa0e. --- Lib/test/test_typing.py | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/Lib/test/test_typing.py b/Lib/test/test_typing.py index 012733eb60d688..709601ff162639 100644 --- a/Lib/test/test_typing.py +++ b/Lib/test/test_typing.py @@ -8101,23 +8101,6 @@ class T4(TypedDict, Generic[S]): pass self.assertEqual(klass.__optional_keys__, set()) self.assertIsInstance(klass(), dict) - def test_setname_called_on_things_in_class_namespace(self): - class CustomException(Exception): pass - - class Annoying: - def __set_name__(self, owner, name): - raise CustomException("Cannot do that!") - - with self.assertRaisesRegex(CustomException, "Cannot do that!") as cm: - class Foo(TypedDict): - attr = Annoying() - - expected_note = ( - "Error calling __set_name__ on 'Annoying' instance " - "'attr' in 'Foo'" - ) - self.assertIn(expected_note, cm.exception.__notes__) - class RequiredTests(BaseTestCase): From e0c88f0a22709805cb8c8edf530117264fc24305 Mon Sep 17 00:00:00 2001 From: AlexWaygood Date: Mon, 13 Nov 2023 15:38:45 +0000 Subject: [PATCH 05/15] Exception -> BaseException --- Lib/test/test_typing.py | 2 +- Lib/typing.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_typing.py b/Lib/test/test_typing.py index 709601ff162639..6504991c448095 100644 --- a/Lib/test/test_typing.py +++ b/Lib/test/test_typing.py @@ -7534,7 +7534,7 @@ class Foo(NamedTuple): self.assertEqual(foo.attr.name, "attr") def test_setname_raises_the_same_as_on_other_classes(self): - class CustomException(Exception): pass + class CustomException(BaseException): pass class Annoying: def __set_name__(self, owner, name): diff --git a/Lib/typing.py b/Lib/typing.py index b08544db4994d7..a33e8478b9f0f3 100644 --- a/Lib/typing.py +++ b/Lib/typing.py @@ -2734,7 +2734,7 @@ def __new__(cls, typename, bases, ns): if hasattr(type(val), "__set_name__"): try: type(val).__set_name__(val, nm_tpl, key) - except Exception as e: + except BaseException as e: e.add_note( f"Error calling __set_name__ on {type(val).__name__!r} " f"instance {key!r} in {typename!r}" From d8102fda3d89d2b1d62d2405d45e0727945471d6 Mon Sep 17 00:00:00 2001 From: AlexWaygood Date: Thu, 16 Nov 2023 07:41:30 +0000 Subject: [PATCH 06/15] Address review (and fix bug) --- Lib/test/test_typing.py | 11 ++++++++++- Lib/typing.py | 14 ++++++++++---- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/Lib/test/test_typing.py b/Lib/test/test_typing.py index 6504991c448095..4300ccb3248a2e 100644 --- a/Lib/test/test_typing.py +++ b/Lib/test/test_typing.py @@ -7519,7 +7519,7 @@ class GenericNamedTuple(NamedTuple, Generic[T]): self.assertEqual(CallNamedTuple.__orig_bases__, (NamedTuple,)) - def test_setname_called_on_non_members(self): + def test_setname_called_on_values_in_class_dictionary(self): class Vanilla: def __set_name__(self, owner, name): self.name = name @@ -7533,6 +7533,15 @@ class Foo(NamedTuple): self.assertIsInstance(foo.attr, Vanilla) self.assertEqual(foo.attr.name, "attr") + class Bar(NamedTuple): + attr: Vanilla = Vanilla() + + bar = Bar() + self.assertEqual(len(bar), 1) + self.assertIn('attr', Bar._fields) + self.assertIsInstance(bar.attr, Vanilla) + self.assertEqual(bar.attr.name, "attr") + def test_setname_raises_the_same_as_on_other_classes(self): class CustomException(BaseException): pass diff --git a/Lib/typing.py b/Lib/typing.py index a33e8478b9f0f3..a7c85f0854b006 100644 --- a/Lib/typing.py +++ b/Lib/typing.py @@ -2729,17 +2729,23 @@ def __new__(cls, typename, bases, ns): for key, val in ns.items(): if key in _prohibited: raise AttributeError("Cannot overwrite NamedTuple attribute " + key) - elif key not in _special and key not in nm_tpl._fields: - setattr(nm_tpl, key, val) - if hasattr(type(val), "__set_name__"): + elif key not in _special: + if key not in nm_tpl._fields: + setattr(nm_tpl, key, val) + try: + set_name = type(val).__set_name__ + except AttributeError: + pass + else: try: - type(val).__set_name__(val, nm_tpl, key) + set_name(val, nm_tpl, key) except BaseException as e: e.add_note( f"Error calling __set_name__ on {type(val).__name__!r} " f"instance {key!r} in {typename!r}" ) raise + if Generic in bases: nm_tpl.__init_subclass__() return nm_tpl From f2a57b2b4a430a3dbbd439e36b5edd2fa38de5d2 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Fri, 17 Nov 2023 15:44:16 +0000 Subject: [PATCH 07/15] Update Lib/test/test_typing.py Co-authored-by: Jelle Zijlstra --- Lib/test/test_typing.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_typing.py b/Lib/test/test_typing.py index 4300ccb3248a2e..d0de7b4430a09e 100644 --- a/Lib/test/test_typing.py +++ b/Lib/test/test_typing.py @@ -7555,7 +7555,7 @@ class NormalClass: normal_exception = cm.exception with self.assertRaisesRegex(CustomException, "Cannot do that!") as cm: - class NamedTupleClass: + class NamedTupleClass(NamedTuple): attr = Annoying() namedtuple_exception = cm.exception From 1909fd59c40a63de3324a22ec18ef59917f6c1dc Mon Sep 17 00:00:00 2001 From: AlexWaygood Date: Fri, 17 Nov 2023 15:57:26 +0000 Subject: [PATCH 08/15] Handle strange exceptions raised on attempting to access `__set_name__` --- Lib/test/test_typing.py | 16 ++++++++++++++++ Lib/typing.py | 2 +- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_typing.py b/Lib/test/test_typing.py index d0de7b4430a09e..0b53017a7df11a 100644 --- a/Lib/test/test_typing.py +++ b/Lib/test/test_typing.py @@ -7577,6 +7577,22 @@ class NamedTupleClass(NamedTuple): normal_exception.__notes__[0].replace("NormalClass", "NamedTupleClass") ) + def test_exception_raised_if_accessing_set_name_causes_strange_error(self): + class Meta(type): + def __getattribute__(self, attr): + if attr == "__set_name__": + raise TypeError("NO") + + class VeryAnnoying(metaclass=Meta): pass + + # Sanity check to make sure the test is setup correctly: + with self.assertRaises(TypeError): + VeryAnnoying.__set_name__ + + # The real test here is just that this class creation succeeds: + class Foo(NamedTuple): + attr = VeryAnnoying() + class TypedDictTests(BaseTestCase): def test_basics_functional_syntax(self): diff --git a/Lib/typing.py b/Lib/typing.py index a7c85f0854b006..4c37bbd19e86af 100644 --- a/Lib/typing.py +++ b/Lib/typing.py @@ -2734,7 +2734,7 @@ def __new__(cls, typename, bases, ns): setattr(nm_tpl, key, val) try: set_name = type(val).__set_name__ - except AttributeError: + except Exception: pass else: try: From ed53fcbed8b12ce399a080766da0c09fe8426950 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Fri, 17 Nov 2023 16:12:33 +0000 Subject: [PATCH 09/15] typo --- Lib/test/test_typing.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_typing.py b/Lib/test/test_typing.py index 0b53017a7df11a..2b384d85178d24 100644 --- a/Lib/test/test_typing.py +++ b/Lib/test/test_typing.py @@ -7577,7 +7577,7 @@ class NamedTupleClass(NamedTuple): normal_exception.__notes__[0].replace("NormalClass", "NamedTupleClass") ) - def test_exception_raised_if_accessing_set_name_causes_strange_error(self): + def test_no_exception_raised_if_accessing_set_name_causes_strange_error(self): class Meta(type): def __getattribute__(self, attr): if attr == "__set_name__": From c2c65c681dedae1446fcbfb840a274f915411b87 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Fri, 17 Nov 2023 16:21:11 +0000 Subject: [PATCH 10/15] big comment --- Lib/typing.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Lib/typing.py b/Lib/typing.py index 4c37bbd19e86af..fc257ecedf5c70 100644 --- a/Lib/typing.py +++ b/Lib/typing.py @@ -2734,6 +2734,11 @@ def __new__(cls, typename, bases, ns): setattr(nm_tpl, key, val) try: set_name = type(val).__set_name__ + # Emulate as much as possible the behaviour of type_new_set_names + # in Objects/typeobject.c: if any exception is raised + # while attempting to access a __set_name__ attribute, + # swallow the exception and move on. + # (We could possibly catch BaseException here, but that feels wrong.) except Exception: pass else: From e79ea37c93878d1fcb3c7a93a8312064b508ac8f Mon Sep 17 00:00:00 2001 From: AlexWaygood Date: Mon, 27 Nov 2023 13:11:49 +0000 Subject: [PATCH 11/15] Revert "big comment" This reverts commit c2c65c681dedae1446fcbfb840a274f915411b87. --- Lib/typing.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/Lib/typing.py b/Lib/typing.py index fc257ecedf5c70..4c37bbd19e86af 100644 --- a/Lib/typing.py +++ b/Lib/typing.py @@ -2734,11 +2734,6 @@ def __new__(cls, typename, bases, ns): setattr(nm_tpl, key, val) try: set_name = type(val).__set_name__ - # Emulate as much as possible the behaviour of type_new_set_names - # in Objects/typeobject.c: if any exception is raised - # while attempting to access a __set_name__ attribute, - # swallow the exception and move on. - # (We could possibly catch BaseException here, but that feels wrong.) except Exception: pass else: From 9fd8b6b5c1c81422a94f52ed285aade4c2de8441 Mon Sep 17 00:00:00 2001 From: AlexWaygood Date: Mon, 27 Nov 2023 13:12:01 +0000 Subject: [PATCH 12/15] Revert "typo" This reverts commit ed53fcbed8b12ce399a080766da0c09fe8426950. --- Lib/test/test_typing.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_typing.py b/Lib/test/test_typing.py index 2b384d85178d24..0b53017a7df11a 100644 --- a/Lib/test/test_typing.py +++ b/Lib/test/test_typing.py @@ -7577,7 +7577,7 @@ class NamedTupleClass(NamedTuple): normal_exception.__notes__[0].replace("NormalClass", "NamedTupleClass") ) - def test_no_exception_raised_if_accessing_set_name_causes_strange_error(self): + def test_exception_raised_if_accessing_set_name_causes_strange_error(self): class Meta(type): def __getattribute__(self, attr): if attr == "__set_name__": From fdab495746a90de59c8d213949abfb1f3ba3b16c Mon Sep 17 00:00:00 2001 From: AlexWaygood Date: Mon, 27 Nov 2023 13:12:16 +0000 Subject: [PATCH 13/15] Revert "Handle strange exceptions raised on attempting to access `__set_name__`" This reverts commit 1909fd59c40a63de3324a22ec18ef59917f6c1dc. --- Lib/test/test_typing.py | 16 ---------------- Lib/typing.py | 2 +- 2 files changed, 1 insertion(+), 17 deletions(-) diff --git a/Lib/test/test_typing.py b/Lib/test/test_typing.py index 0b53017a7df11a..d0de7b4430a09e 100644 --- a/Lib/test/test_typing.py +++ b/Lib/test/test_typing.py @@ -7577,22 +7577,6 @@ class NamedTupleClass(NamedTuple): normal_exception.__notes__[0].replace("NormalClass", "NamedTupleClass") ) - def test_exception_raised_if_accessing_set_name_causes_strange_error(self): - class Meta(type): - def __getattribute__(self, attr): - if attr == "__set_name__": - raise TypeError("NO") - - class VeryAnnoying(metaclass=Meta): pass - - # Sanity check to make sure the test is setup correctly: - with self.assertRaises(TypeError): - VeryAnnoying.__set_name__ - - # The real test here is just that this class creation succeeds: - class Foo(NamedTuple): - attr = VeryAnnoying() - class TypedDictTests(BaseTestCase): def test_basics_functional_syntax(self): diff --git a/Lib/typing.py b/Lib/typing.py index 4c37bbd19e86af..a7c85f0854b006 100644 --- a/Lib/typing.py +++ b/Lib/typing.py @@ -2734,7 +2734,7 @@ def __new__(cls, typename, bases, ns): setattr(nm_tpl, key, val) try: set_name = type(val).__set_name__ - except Exception: + except AttributeError: pass else: try: From 3932d485b629694afb029bd9df29da012c70ee72 Mon Sep 17 00:00:00 2001 From: AlexWaygood Date: Mon, 27 Nov 2023 13:15:51 +0000 Subject: [PATCH 14/15] Add a test that asserts the correct behaviour --- Lib/test/test_typing.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/Lib/test/test_typing.py b/Lib/test/test_typing.py index d0de7b4430a09e..0303515051ebc2 100644 --- a/Lib/test/test_typing.py +++ b/Lib/test/test_typing.py @@ -7577,6 +7577,21 @@ class NamedTupleClass(NamedTuple): normal_exception.__notes__[0].replace("NormalClass", "NamedTupleClass") ) + def test_strange_errors_when_accessing_set_name_itself(self): + class Meta(type): + def __getattribute__(self, attr): + if attr == "__set_name__": + raise TypeError("NO") + return object.__getattribute__(self, attr) + + class VeryAnnoying(metaclass=Meta): pass + + annoying = VeryAnnoying() + + with self.assertRaisesRegex(TypeError, "NO"): + class Foo(NamedTuple): + attr = annoying + class TypedDictTests(BaseTestCase): def test_basics_functional_syntax(self): From 3359ff60b8b41a20e276ea828bec5e279049ff27 Mon Sep 17 00:00:00 2001 From: AlexWaygood Date: Mon, 27 Nov 2023 16:14:42 +0000 Subject: [PATCH 15/15] Address Serhiy's second review pass --- Lib/test/test_typing.py | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/Lib/test/test_typing.py b/Lib/test/test_typing.py index 0303515051ebc2..f4e3ff0fd22a81 100644 --- a/Lib/test/test_typing.py +++ b/Lib/test/test_typing.py @@ -7547,16 +7547,18 @@ class CustomException(BaseException): pass class Annoying: def __set_name__(self, owner, name): - raise CustomException("Cannot do that!") + raise CustomException - with self.assertRaisesRegex(CustomException, "Cannot do that!") as cm: + annoying = Annoying() + + with self.assertRaises(CustomException) as cm: class NormalClass: - attr = Annoying() + attr = annoying normal_exception = cm.exception - with self.assertRaisesRegex(CustomException, "Cannot do that!") as cm: + with self.assertRaises(CustomException) as cm: class NamedTupleClass(NamedTuple): - attr = Annoying() + attr = annoying namedtuple_exception = cm.exception self.assertIs(type(namedtuple_exception), CustomException) @@ -7578,19 +7580,21 @@ class NamedTupleClass(NamedTuple): ) def test_strange_errors_when_accessing_set_name_itself(self): + class CustomException(Exception): pass + class Meta(type): def __getattribute__(self, attr): if attr == "__set_name__": - raise TypeError("NO") + raise CustomException return object.__getattribute__(self, attr) class VeryAnnoying(metaclass=Meta): pass - annoying = VeryAnnoying() + very_annoying = VeryAnnoying() - with self.assertRaisesRegex(TypeError, "NO"): + with self.assertRaises(CustomException): class Foo(NamedTuple): - attr = annoying + attr = very_annoying class TypedDictTests(BaseTestCase):