From f3fcea65e84f10ab7790fe79e559043e158bfd6e Mon Sep 17 00:00:00 2001 From: "Eric V. Smith" Date: Mon, 19 Mar 2018 21:44:31 -0400 Subject: [PATCH 1/7] Add failing test. --- Lib/test/test_dataclasses.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/Lib/test/test_dataclasses.py b/Lib/test/test_dataclasses.py index db03ec1925f674..4417bfd027172e 100755 --- a/Lib/test/test_dataclasses.py +++ b/Lib/test/test_dataclasses.py @@ -1147,6 +1147,24 @@ class C: C().x self.assertEqual(factory.call_count, 2) + def test_default_factory_derived(self): + # See bpo-32896. + @dataclass + class Foo: + x: dict = field(default_factory=dict) + + @dataclass + class Bar(Foo): + y: int = 1 + + self.assertEqual(Foo().x, {}) + self.assertEqual(Bar().x, {}) + + @dataclass + class Baz(Foo): + pass + Baz() + def x_test_classvar_default_factory(self): # XXX: it's an error for a ClassVar to have a factory function @dataclass From 3f29371a7069cb4904e6ba13f6c3af70c029b319 Mon Sep 17 00:00:00 2001 From: "Eric V. Smith" Date: Tue, 20 Mar 2018 05:15:10 -0400 Subject: [PATCH 2/7] Checkpoint. --- Lib/dataclasses.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/Lib/dataclasses.py b/Lib/dataclasses.py index a4afd50376bdc6..61031adc9fa6c7 100644 --- a/Lib/dataclasses.py +++ b/Lib/dataclasses.py @@ -318,6 +318,11 @@ def _create_fn(name, args, body, *, globals=None, locals=None, # Compute the text of the entire function. txt = f'def {name}({args}){return_annotation}:\n{body}' + if name == '__init__': + print(txt) + print(globals) + print(locals) + print() exec(txt, globals, locals) return locals[name] @@ -661,10 +666,14 @@ def _process_class(cls, init, repr, eq, order, unsafe_hash, frozen): if getattr(b, _PARAMS).frozen: any_frozen_base = True + print(f'found fields in {cls.__name__}: {fields}') + print('annotations', cls.__annotations__) + print('dict', cls.__dict__.keys()) # Now find fields in our class. While doing so, validate some # things, and set the default values (as class attributes) # where we can. for f in _find_fields(cls): + print('adding field', f) fields[f.name] = f # If the class attribute (which is the default value for From 3edbe4629f441e6d2f8f4163814b476a0bc3becd Mon Sep 17 00:00:00 2001 From: "Eric V. Smith" Date: Tue, 20 Mar 2018 20:42:21 -0400 Subject: [PATCH 3/7] Checkpoint. --- Lib/dataclasses.py | 32 +++++++++++++------------------- 1 file changed, 13 insertions(+), 19 deletions(-) diff --git a/Lib/dataclasses.py b/Lib/dataclasses.py index 61031adc9fa6c7..fcc4304ecb75c4 100644 --- a/Lib/dataclasses.py +++ b/Lib/dataclasses.py @@ -318,11 +318,6 @@ def _create_fn(name, args, body, *, globals=None, locals=None, # Compute the text of the entire function. txt = f'def {name}({args}){return_annotation}:\n{body}' - if name == '__init__': - print(txt) - print(globals) - print(locals) - print() exec(txt, globals, locals) return locals[name] @@ -579,17 +574,20 @@ def _get_field(cls, a_name, a_type): def _find_fields(cls): # Return a list of Field objects, in order, for this class (and no - # base classes). Fields are found from __annotations__ (which is - # guaranteed to be ordered). Default values are from class - # attributes, if a field has a default. If the default value is - # a Field(), then it contains additional info beyond (and - # possibly including) the actual default value. Pseudo-fields - # ClassVars and InitVars are included, despite the fact that - # they're not real fields. That's dealt with later. + # base classes). Fields are found from the class dict's + # __annotations__ (which is guaranteed to be ordered). Default + # values are from class attributes, if a field has a default. If + # the default value is a Field(), then it contains additional + # info beyond (and possibly including) the actual default value. + # Pseudo-fields ClassVars and InitVars are included, despite the + # fact that they're not real fields. That's dealt with later. - annotations = getattr(cls, '__annotations__', {}) - return [_get_field(cls, a_name, a_type) - for a_name, a_type in annotations.items()] + try: + annotations = cls.__dict__['__annotations__'] + except KeyError: + # This class defines no annotations. + annotations = {} + return [_get_field(cls, name, type) for name, type in annotations.items()] def _set_new_attribute(cls, name, value): @@ -666,14 +664,10 @@ def _process_class(cls, init, repr, eq, order, unsafe_hash, frozen): if getattr(b, _PARAMS).frozen: any_frozen_base = True - print(f'found fields in {cls.__name__}: {fields}') - print('annotations', cls.__annotations__) - print('dict', cls.__dict__.keys()) # Now find fields in our class. While doing so, validate some # things, and set the default values (as class attributes) # where we can. for f in _find_fields(cls): - print('adding field', f) fields[f.name] = f # If the class attribute (which is the default value for From beca250ef38d0e258faff99933ed78da20bfe6ac Mon Sep 17 00:00:00 2001 From: "Eric V. Smith" Date: Tue, 20 Mar 2018 20:47:20 -0400 Subject: [PATCH 4/7] Add more tests for intermediate classes not defining fields. --- Lib/test/test_dataclasses.py | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/Lib/test/test_dataclasses.py b/Lib/test/test_dataclasses.py index 4417bfd027172e..613e1d0f76b1aa 100755 --- a/Lib/test/test_dataclasses.py +++ b/Lib/test/test_dataclasses.py @@ -1165,6 +1165,36 @@ class Baz(Foo): pass Baz() + def test_intermediate_non_dataclass(self): + # Test that an intermediate class that defines + # annotations does not define fields. + + @dataclass + class A: + x: int + + class B(A): + y: int + + @dataclass + class C(B): + z: int + + c = C(1, 3) + self.assertEqual((c.x, c.z), (1, 3)) + + # .y was not initialized. + with self.assertRaisesRegex(AttributeError, + 'object has no attribute'): + c.y + + # And if we again derive a non-dataclass, no fields are added. + class D(C): + t: int + d = D(4, 5) + self.assertEqual((d.x, d.z), (4, 5)) + + def x_test_classvar_default_factory(self): # XXX: it's an error for a ClassVar to have a factory function @dataclass From ddfc9faa15db59bca3c4245a3316ca6d222129a5 Mon Sep 17 00:00:00 2001 From: "Eric V. Smith" Date: Tue, 20 Mar 2018 20:49:05 -0400 Subject: [PATCH 5/7] Added more tests. --- Lib/test/test_dataclasses.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_dataclasses.py b/Lib/test/test_dataclasses.py index 613e1d0f76b1aa..9b5aad25745f7f 100755 --- a/Lib/test/test_dataclasses.py +++ b/Lib/test/test_dataclasses.py @@ -1159,11 +1159,12 @@ class Bar(Foo): self.assertEqual(Foo().x, {}) self.assertEqual(Bar().x, {}) + self.assertEqual(Bar().y, 1) @dataclass class Baz(Foo): pass - Baz() + self.assertEqual(Baz().x, {}) def test_intermediate_non_dataclass(self): # Test that an intermediate class that defines From 0ed4d7444a8da0c320b0eeb1feacdb3a0c18c543 Mon Sep 17 00:00:00 2001 From: "Eric V. Smith" Date: Tue, 20 Mar 2018 20:53:53 -0400 Subject: [PATCH 6/7] Improve a comment. Add blurb. --- Lib/dataclasses.py | 2 +- .../next/Library/2018-03-20-20-53-21.bpo-32896.ewW3Ln.rst | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) create mode 100644 Misc/NEWS.d/next/Library/2018-03-20-20-53-21.bpo-32896.ewW3Ln.rst diff --git a/Lib/dataclasses.py b/Lib/dataclasses.py index fcc4304ecb75c4..3d97782c80f24e 100644 --- a/Lib/dataclasses.py +++ b/Lib/dataclasses.py @@ -585,7 +585,7 @@ def _find_fields(cls): try: annotations = cls.__dict__['__annotations__'] except KeyError: - # This class defines no annotations. + # This class defines no additional annotations. annotations = {} return [_get_field(cls, name, type) for name, type in annotations.items()] diff --git a/Misc/NEWS.d/next/Library/2018-03-20-20-53-21.bpo-32896.ewW3Ln.rst b/Misc/NEWS.d/next/Library/2018-03-20-20-53-21.bpo-32896.ewW3Ln.rst new file mode 100644 index 00000000000000..8363da4667a180 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2018-03-20-20-53-21.bpo-32896.ewW3Ln.rst @@ -0,0 +1,2 @@ +Fix an error where subclassing a dataclass with a field that uses a +default_factory would generate an incorrect class. From ad0f534b7701f1ae4366fd9782525936c4e47668 Mon Sep 17 00:00:00 2001 From: "Eric V. Smith" Date: Tue, 20 Mar 2018 21:14:21 -0400 Subject: [PATCH 7/7] Improve dict lookup. --- Lib/dataclasses.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/Lib/dataclasses.py b/Lib/dataclasses.py index 3d97782c80f24e..d6164324914889 100644 --- a/Lib/dataclasses.py +++ b/Lib/dataclasses.py @@ -582,11 +582,9 @@ def _find_fields(cls): # Pseudo-fields ClassVars and InitVars are included, despite the # fact that they're not real fields. That's dealt with later. - try: - annotations = cls.__dict__['__annotations__'] - except KeyError: - # This class defines no additional annotations. - annotations = {} + # If __annotations__ isn't present, then this class adds no new + # annotations. + annotations = cls.__dict__.get('__annotations__', {}) return [_get_field(cls, name, type) for name, type in annotations.items()]