From db3eef761167190464708071484a39b7c4ae02db Mon Sep 17 00:00:00 2001 From: Erik De Bonte Date: Wed, 30 Mar 2022 10:15:37 -0700 Subject: [PATCH 01/10] First cut at unit tests --- Lib/test/test_dataclasses.py | 49 ++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/Lib/test/test_dataclasses.py b/Lib/test/test_dataclasses.py index 98dfab481bfd14..1e92bc8e5777a3 100644 --- a/Lib/test/test_dataclasses.py +++ b/Lib/test/test_dataclasses.py @@ -3229,6 +3229,55 @@ class C: self.assertEqual(D.__set_name__.call_count, 1) + def test_init_calls_set(self): + class D: + pass + + D.__set__ = Mock() + + @dataclass + class C: + i: D = D() + + # Make sure D.__set__ is called. + D.__set__.reset_mock() + c = C(5) + self.assertEqual(D.__set__.call_count, 1) + + def test_getting_field_calls_get(self): + class D: + pass + + D.__set__ = Mock() + D.__get__ = Mock() + + @dataclass + class C: + i: D = D() + + c = C(5) + + # Make sure D.__set__ is called. + D.__get__.reset_mock() + value = c.i + self.assertEqual(D.__get__.call_count, 1) + + def test_setting_field_calls_set(self): + class D: + pass + + D.__set__ = Mock() + + @dataclass + class C: + i: D = D() + + c = C(5) + + # Make sure D.__set__ is called. + D.__set__.reset_mock() + c.i = 10 + self.assertEqual(D.__set__.call_count, 1) class TestStringAnnotations(unittest.TestCase): def test_classvar(self): From 8b4689ffd9e1d2b544554b3aea41638f125d4939 Mon Sep 17 00:00:00 2001 From: Erik De Bonte Date: Mon, 27 Jun 2022 16:34:15 -0700 Subject: [PATCH 02/10] Fix comment typo --- Lib/test/test_dataclasses.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_dataclasses.py b/Lib/test/test_dataclasses.py index 1e92bc8e5777a3..1b7d86014aad82 100644 --- a/Lib/test/test_dataclasses.py +++ b/Lib/test/test_dataclasses.py @@ -3257,7 +3257,7 @@ class C: c = C(5) - # Make sure D.__set__ is called. + # Make sure D.__get__ is called. D.__get__.reset_mock() value = c.i self.assertEqual(D.__get__.call_count, 1) From 1ea97fab603cd439f36ae6bd683128ef51e4c984 Mon Sep 17 00:00:00 2001 From: Erik De Bonte Date: Mon, 27 Jun 2022 18:55:02 -0700 Subject: [PATCH 03/10] Add tests for uninitialized field and default value --- Lib/test/test_dataclasses.py | 41 ++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/Lib/test/test_dataclasses.py b/Lib/test/test_dataclasses.py index 1b7d86014aad82..817d1f374a0b59 100644 --- a/Lib/test/test_dataclasses.py +++ b/Lib/test/test_dataclasses.py @@ -3279,6 +3279,47 @@ class C: c.i = 10 self.assertEqual(D.__set__.call_count, 1) + def test_setting_uninitialized_descriptor_field(self): + class D: + pass + + D.__set__ = Mock() + + @dataclass + class C: + i: D + + # D.__set__ is not called because there's no D instance to call it on + D.__set__.reset_mock() + c = C(5) + self.assertEqual(D.__set__.call_count, 0) + + # D.__set__ still isn't called after setting i to an instance of D + # because descriptors don't behave like that when stored as instance vars + c.i = D() + c.i = 5 + self.assertEqual(D.__set__.call_count, 0) + + def test_default_value(self): + class D: + def __get__(self, instance: Any, owner: object) -> int: + if instance is None: + return 100 + return instance._x + + def __set__(self, instance: Any, value: int) -> None: + instance._x = value + + @dataclass + class C: + i: D = D() + + c = C() + self.assertEqual(c.i, 100) + + c = C(5) + self.assertEqual(c.i, 5) + class TestStringAnnotations(unittest.TestCase): def test_classvar(self): # Some expressions recognized as ClassVar really aren't. But From 409614c8166e2091d408484a3e7441fa0207b679 Mon Sep 17 00:00:00 2001 From: Erik De Bonte Date: Wed, 29 Jun 2022 08:21:01 -0700 Subject: [PATCH 04/10] Add test for __get__ raising AttributeError, no default val --- Lib/test/test_dataclasses.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/Lib/test/test_dataclasses.py b/Lib/test/test_dataclasses.py index 817d1f374a0b59..cfa82093dfbe00 100644 --- a/Lib/test/test_dataclasses.py +++ b/Lib/test/test_dataclasses.py @@ -3305,6 +3305,7 @@ class D: def __get__(self, instance: Any, owner: object) -> int: if instance is None: return 100 + return instance._x def __set__(self, instance: Any, value: int) -> None: @@ -3320,6 +3321,24 @@ class C: c = C(5) self.assertEqual(c.i, 5) + def test_no_default_value(self): + class D: + def __get__(self, instance: Any, owner: object) -> int: + if instance is None: + raise AttributeError() + + return instance._x + + def __set__(self, instance: Any, value: int) -> None: + instance._x = value + + @dataclass + class C: + i: D = D() + + with self.assertRaisesRegex(TypeError, 'missing 1 required positional argument'): + c = C() + class TestStringAnnotations(unittest.TestCase): def test_classvar(self): # Some expressions recognized as ClassVar really aren't. But From ad246baf7c3904b771a62395048c526814144549 Mon Sep 17 00:00:00 2001 From: Erik De Bonte Date: Wed, 29 Jun 2022 09:19:02 -0700 Subject: [PATCH 05/10] First cut at docs --- Doc/library/dataclasses.rst | 44 +++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/Doc/library/dataclasses.rst b/Doc/library/dataclasses.rst index ec50696ea89d40..a40208fc5bfe95 100644 --- a/Doc/library/dataclasses.rst +++ b/Doc/library/dataclasses.rst @@ -749,3 +749,47 @@ mutable types as default values for fields:: ``dict``, or ``set``, unhashable objects are now not allowed as default values. Unhashability is used to approximate mutability. + +Descriptor-typed fields +----------------------- + +Fields that are assigned :ref:`descriptor objects ` as their +default value have the following special behaviors: + +* The value for the field passed to the dataclass's ``__init__`` method is + passed to the descriptor's ``__set__`` method rather than overwriting the + descriptor object. +* Similiarly, when getting or setting the field, the descriptor's + ``__get__`` or ``__set__`` method is called rather than returning or + overwriting the descriptor object. +* The value returned by the descriptor's ``__get__`` method when its + ``obj`` parameter is ``None``, will be used as the field's default + value. If the descriptor raises an ``AttributeError`` when ``obj`` + is ``None``, the field will not have a default value. + +:: + + class Descriptor(): + def __init__(self, *, default): + self._default = default + + def __get__(self, obj, objtype): + if obj is None: + return self._default + + return getattr(obj, "_x") + + def __set__(self, obj, value): + if obj is not None: + setattr(obj, "_x", value) + + @dataclass + class InventoryItem: + quantity_on_hand: Descriptor = Descriptor(default=100) + + i = InventoryItem() + print(i.quantity_on_hand) # 100 + +Note that if a field is annotated with a desriptor type, but is not assigned +a descriptor object as its default value, the field will act like a normal +field. From e2e66f928552648052d3411e1e06430a35914ab5 Mon Sep 17 00:00:00 2001 From: Erik De Bonte Date: Wed, 29 Jun 2022 12:18:32 -0700 Subject: [PATCH 06/10] Apply suggestions from ambv MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Łukasz Langa --- Doc/library/dataclasses.rst | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/Doc/library/dataclasses.rst b/Doc/library/dataclasses.rst index a40208fc5bfe95..54b4bd1f833e69 100644 --- a/Doc/library/dataclasses.rst +++ b/Doc/library/dataclasses.rst @@ -759,25 +759,28 @@ default value have the following special behaviors: * The value for the field passed to the dataclass's ``__init__`` method is passed to the descriptor's ``__set__`` method rather than overwriting the descriptor object. -* Similiarly, when getting or setting the field, the descriptor's +* Similarly, when getting or setting the field, the descriptor's ``__get__`` or ``__set__`` method is called rather than returning or overwriting the descriptor object. -* The value returned by the descriptor's ``__get__`` method when its - ``obj`` parameter is ``None``, will be used as the field's default - value. If the descriptor raises an ``AttributeError`` when ``obj`` - is ``None``, the field will not have a default value. +* To determine whether a field contains a default value, ``dataclasses`` + will call the descriptor's ``__get__`` method using its class access + form (i.e. ``descriptor.__get__(obj=None, type=cls)``. If the + descriptor returns a value in this case, it will be used as the + field's default. On the other hand, if the descriptor raises + :exc:`AttributeError` in this situation, no default value will be + provided for the field. :: - class Descriptor(): + class Descriptor: def __init__(self, *, default): self._default = default - def __get__(self, obj, objtype): + def __get__(self, obj, type): if obj is None: return self._default - return getattr(obj, "_x") + return getattr(obj, "_x", self._default) def __set__(self, obj, value): if obj is not None: @@ -790,6 +793,6 @@ default value have the following special behaviors: i = InventoryItem() print(i.quantity_on_hand) # 100 -Note that if a field is annotated with a desriptor type, but is not assigned +Note that if a field is annotated with a descriptor type, but is not assigned a descriptor object as its default value, the field will act like a normal field. From 5a921495a3cae28d97a78f228f5a623084659bc9 Mon Sep 17 00:00:00 2001 From: Erik De Bonte Date: Wed, 29 Jun 2022 12:19:41 -0700 Subject: [PATCH 07/10] Remove None check in __set__ --- Doc/library/dataclasses.rst | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Doc/library/dataclasses.rst b/Doc/library/dataclasses.rst index 54b4bd1f833e69..f3420c91c5be12 100644 --- a/Doc/library/dataclasses.rst +++ b/Doc/library/dataclasses.rst @@ -783,8 +783,7 @@ default value have the following special behaviors: return getattr(obj, "_x", self._default) def __set__(self, obj, value): - if obj is not None: - setattr(obj, "_x", value) + setattr(obj, "_x", value) @dataclass class InventoryItem: From 6432ee33a3b6ab16e41dc264bcd5f7a0fe2bef40 Mon Sep 17 00:00:00 2001 From: Erik De Bonte Date: Wed, 29 Jun 2022 13:29:05 -0700 Subject: [PATCH 08/10] Use __set_name__ instead of hardcoded name --- Doc/library/dataclasses.rst | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/Doc/library/dataclasses.rst b/Doc/library/dataclasses.rst index f3420c91c5be12..b3ca4df1820608 100644 --- a/Doc/library/dataclasses.rst +++ b/Doc/library/dataclasses.rst @@ -776,14 +776,17 @@ default value have the following special behaviors: def __init__(self, *, default): self._default = default + def __set_name__(self, owner, name): + self._name = "_" + name + def __get__(self, obj, type): if obj is None: return self._default - return getattr(obj, "_x", self._default) + return getattr(obj, self._name, self._default) def __set__(self, obj, value): - setattr(obj, "_x", value) + setattr(obj, self._name, value) @dataclass class InventoryItem: From 6e19aef9da9c62aef4d2a5ba79f42cc568b5ce48 Mon Sep 17 00:00:00 2001 From: Erik De Bonte Date: Wed, 29 Jun 2022 13:34:38 -0700 Subject: [PATCH 09/10] Attempt at more realistic example --- Doc/library/dataclasses.rst | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/Doc/library/dataclasses.rst b/Doc/library/dataclasses.rst index b3ca4df1820608..4364ac342471eb 100644 --- a/Doc/library/dataclasses.rst +++ b/Doc/library/dataclasses.rst @@ -772,7 +772,7 @@ default value have the following special behaviors: :: - class Descriptor: + class IntConversionDescriptor: def __init__(self, *, default): self._default = default @@ -786,14 +786,16 @@ default value have the following special behaviors: return getattr(obj, self._name, self._default) def __set__(self, obj, value): - setattr(obj, self._name, value) + setattr(obj, self._name, int(value)) @dataclass class InventoryItem: - quantity_on_hand: Descriptor = Descriptor(default=100) + quantity_on_hand: IntConversionDescriptor = IntConversionDescriptor(default=100) i = InventoryItem() print(i.quantity_on_hand) # 100 + i.quantity_on_hand = 2.5 # calls __set__ with 2.5 + print(i.quantity_on_hand) # 2 Note that if a field is annotated with a descriptor type, but is not assigned a descriptor object as its default value, the field will act like a normal From ad9f6f4730d8816a9d004dbc951f1ac96b91edbf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Langa?= Date: Tue, 5 Jul 2022 17:53:17 +0200 Subject: [PATCH 10/10] Add Blurb --- .../Tests/2022-07-05-17-53-13.gh-issue-91330.Qys5IL.rst | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 Misc/NEWS.d/next/Tests/2022-07-05-17-53-13.gh-issue-91330.Qys5IL.rst diff --git a/Misc/NEWS.d/next/Tests/2022-07-05-17-53-13.gh-issue-91330.Qys5IL.rst b/Misc/NEWS.d/next/Tests/2022-07-05-17-53-13.gh-issue-91330.Qys5IL.rst new file mode 100644 index 00000000000000..315521102f4ec3 --- /dev/null +++ b/Misc/NEWS.d/next/Tests/2022-07-05-17-53-13.gh-issue-91330.Qys5IL.rst @@ -0,0 +1,7 @@ +Added more tests for :mod:`dataclasses` to cover behavior with data +descriptor-based fields. + +# Write your Misc/NEWS entry below. It should be a simple ReST paragraph. # +Don't start with "- Issue #: " or "- gh-issue-: " or that sort of +stuff. +###########################################################################