From ba10ba580fc798c46ede80b96de66e81e5beba46 Mon Sep 17 00:00:00 2001 From: Alex Hall Date: Mon, 29 Jan 2024 23:24:15 +0200 Subject: [PATCH 01/12] Fix inheriting annotations in dataclasses --- pydantic/dataclasses.py | 42 +++++++++++++++++++++-------------------- 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/pydantic/dataclasses.py b/pydantic/dataclasses.py index bb26540cce..41665384c8 100644 --- a/pydantic/dataclasses.py +++ b/pydantic/dataclasses.py @@ -153,26 +153,28 @@ def make_pydantic_fields_compatible(cls: type[Any]) -> None: into `x: int = dataclasses.field(default=pydantic.Field(..., kw_only=True), kw_only=True)` """ - # In Python < 3.9, `__annotations__` might not be present if there are no fields. - # we therefore need to use `getattr` to avoid an `AttributeError`. - for field_name in getattr(cls, '__annotations__', []): - field_value = getattr(cls, field_name, None) - # Process only if this is an instance of `FieldInfo`. - if not isinstance(field_value, FieldInfo): - continue - - # Initialize arguments for the standard `dataclasses.field`. - field_args: dict = {'default': field_value} - - # Handle `kw_only` for Python 3.10+ - if sys.version_info >= (3, 10) and field_value.kw_only: - field_args['kw_only'] = True - - # Set `repr` attribute if it's explicitly specified to be not `True`. - if field_value.repr is not True: - field_args['repr'] = field_value.repr - - setattr(cls, field_name, dataclasses.field(**field_args)) + for annotation_cls in cls.__mro__: + # In Python < 3.9, `__annotations__` might not be present if there are no fields. + # we therefore need to use `getattr` to avoid an `AttributeError`. + for field_name in getattr(annotation_cls, '__annotations__', []): + field_value = getattr(cls, field_name, None) + # Process only if this is an instance of `FieldInfo`. + if not isinstance(field_value, FieldInfo): + continue + + # Initialize arguments for the standard `dataclasses.field`. + field_args: dict = {'default': field_value} + + # Handle `kw_only` for Python 3.10+ + if sys.version_info >= (3, 10) and field_value.kw_only: + field_args['kw_only'] = True + + # Set `repr` attribute if it's explicitly specified to be not `True`. + if field_value.repr is not True: + field_args['repr'] = field_value.repr + + setattr(cls, field_name, dataclasses.field(**field_args)) + cls.__annotations__[field_name] = annotation_cls.__annotations__[field_name] def create_dataclass(cls: type[Any]) -> type[PydanticDataclass]: """Create a Pydantic dataclass from a regular dataclass. From 72728dd3ac112c9577377819a0401d49304bb6bb Mon Sep 17 00:00:00 2001 From: sydney-runkle Date: Tue, 30 Jan 2024 07:43:52 -0600 Subject: [PATCH 02/12] adding test --- tests/test_dataclasses.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/test_dataclasses.py b/tests/test_dataclasses.py index fcf635f8b8..d4b9e12573 100644 --- a/tests/test_dataclasses.py +++ b/tests/test_dataclasses.py @@ -2759,3 +2759,17 @@ def test_disallow_init_false_and_init_var_true() -> None: @pydantic.dataclasses.dataclass class Foo: bar: str = Field(..., init=False, init_var=True) + + +def test_annotations_valid_for_field_inheritance() -> None: + # testing https://github.com/pydantic/pydantic/issues/8670 + + class A: + a: int = pydantic.dataclasses.Field() + + @pydantic.dataclasses.dataclass() + class B(A): + ... + + assert 'a' in B.__pydantic_fields__ + assert 'a' in B.__annotations__ From 571c0c9e79e883491614d2af672b1107087755cc Mon Sep 17 00:00:00 2001 From: sydney-runkle Date: Tue, 30 Jan 2024 07:49:42 -0600 Subject: [PATCH 03/12] update test --- tests/test_dataclasses.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/test_dataclasses.py b/tests/test_dataclasses.py index d4b9e12573..7bb40f5c84 100644 --- a/tests/test_dataclasses.py +++ b/tests/test_dataclasses.py @@ -2771,5 +2771,4 @@ class A: class B(A): ... - assert 'a' in B.__pydantic_fields__ - assert 'a' in B.__annotations__ + assert B.__pydantic_fields__['a'].annotation is int From 2dedb593a5d4aefc26836b8e8966825d82979269 Mon Sep 17 00:00:00 2001 From: sydney-runkle Date: Tue, 30 Jan 2024 10:17:14 -0600 Subject: [PATCH 04/12] fix, improved tests --- pydantic/dataclasses.py | 7 ++++++- tests/test_dataclasses.py | 20 ++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/pydantic/dataclasses.py b/pydantic/dataclasses.py index 41665384c8..66e895cb1d 100644 --- a/pydantic/dataclasses.py +++ b/pydantic/dataclasses.py @@ -153,7 +153,7 @@ def make_pydantic_fields_compatible(cls: type[Any]) -> None: into `x: int = dataclasses.field(default=pydantic.Field(..., kw_only=True), kw_only=True)` """ - for annotation_cls in cls.__mro__: + for annotation_cls in cls.__mro__[::-1]: # In Python < 3.9, `__annotations__` might not be present if there are no fields. # we therefore need to use `getattr` to avoid an `AttributeError`. for field_name in getattr(annotation_cls, '__annotations__', []): @@ -174,6 +174,11 @@ def make_pydantic_fields_compatible(cls: type[Any]) -> None: field_args['repr'] = field_value.repr setattr(cls, field_name, dataclasses.field(**field_args)) + + # In Python 3.8, dataclasses checks cls.__dict__['__annotations__'] for annotations, + # so we must make sure it's initialized before we add to it. + if cls.__dict__.get('__annotations__') is None: + cls.__annotations__ = {} cls.__annotations__[field_name] = annotation_cls.__annotations__[field_name] def create_dataclass(cls: type[Any]) -> type[PydanticDataclass]: diff --git a/tests/test_dataclasses.py b/tests/test_dataclasses.py index 7bb40f5c84..91bf12a4cf 100644 --- a/tests/test_dataclasses.py +++ b/tests/test_dataclasses.py @@ -2772,3 +2772,23 @@ class B(A): ... assert B.__pydantic_fields__['a'].annotation is int + + assert B(a=1).a == 1 + + +def test_annotations_valid_for_field_inheritance_with_existing_field() -> None: + # variation on testing https://github.com/pydantic/pydantic/issues/8670 + + class A: + a: int = pydantic.dataclasses.Field() + + @pydantic.dataclasses.dataclass() + class B(A): + b: str + + assert B.__pydantic_fields__['a'].annotation is int + assert B.__pydantic_fields__['b'].annotation is str + + b = B(a=1, b='b') + assert b.a == 1 + assert b.b == 'b' From 4872a845d3f75158cbab6cfe6176d97b41d76150 Mon Sep 17 00:00:00 2001 From: sydney-runkle Date: Tue, 30 Jan 2024 10:22:03 -0600 Subject: [PATCH 05/12] lint fix --- pydantic/dataclasses.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pydantic/dataclasses.py b/pydantic/dataclasses.py index 66e895cb1d..2f2fec7819 100644 --- a/pydantic/dataclasses.py +++ b/pydantic/dataclasses.py @@ -93,7 +93,7 @@ def dataclass( @dataclass_transform(field_specifiers=(dataclasses.field, Field)) -def dataclass( +def dataclass( # noqa: C901 _cls: type[_T] | None = None, *, init: Literal[False] = False, From 17decda76c4f2a14c6d9dd8fbf9204e9e70e1d05 Mon Sep 17 00:00:00 2001 From: sydney-runkle Date: Tue, 30 Jan 2024 11:07:58 -0600 Subject: [PATCH 06/12] don't reverse mro, add test --- pydantic/dataclasses.py | 2 +- tests/test_dataclasses.py | 20 ++++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/pydantic/dataclasses.py b/pydantic/dataclasses.py index 2f2fec7819..a31fadff74 100644 --- a/pydantic/dataclasses.py +++ b/pydantic/dataclasses.py @@ -153,7 +153,7 @@ def make_pydantic_fields_compatible(cls: type[Any]) -> None: into `x: int = dataclasses.field(default=pydantic.Field(..., kw_only=True), kw_only=True)` """ - for annotation_cls in cls.__mro__[::-1]: + for annotation_cls in cls.__mro__: # In Python < 3.9, `__annotations__` might not be present if there are no fields. # we therefore need to use `getattr` to avoid an `AttributeError`. for field_name in getattr(annotation_cls, '__annotations__', []): diff --git a/tests/test_dataclasses.py b/tests/test_dataclasses.py index 91bf12a4cf..f2edca2503 100644 --- a/tests/test_dataclasses.py +++ b/tests/test_dataclasses.py @@ -2792,3 +2792,23 @@ class B(A): b = B(a=1, b='b') assert b.a == 1 assert b.b == 'b' + + +def test_annotations_with_override() -> None: + class A: + a: int = pydantic.dataclasses.Field() + b: int = pydantic.dataclasses.Field() + c: int + d: int + + @pydantic.dataclasses.dataclass() + class B(A): + a: str = pydantic.dataclasses.Field() + b: str + c: str = pydantic.dataclasses.Field() + d: str + + b = B(a='a', b='b', c='c', d='d') + for field_name in ['a', 'b', 'c', 'd']: + assert B.__pydantic_fields__[field_name].annotation is str + assert getattr(b, field_name) == field_name From 4de0a1a2525196ebeaa4b26e191297186c5d1bc5 Mon Sep 17 00:00:00 2001 From: sydney-runkle Date: Tue, 30 Jan 2024 11:12:16 -0600 Subject: [PATCH 07/12] don't reverse mro, add test --- tests/test_dataclasses.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/test_dataclasses.py b/tests/test_dataclasses.py index f2edca2503..025d53d840 100644 --- a/tests/test_dataclasses.py +++ b/tests/test_dataclasses.py @@ -2796,17 +2796,17 @@ class B(A): def test_annotations_with_override() -> None: class A: - a: int = pydantic.dataclasses.Field() - b: int = pydantic.dataclasses.Field() - c: int - d: int + a: int + b: int + c: int = pydantic.dataclasses.Field() + d: int = pydantic.dataclasses.Field() @pydantic.dataclasses.dataclass() class B(A): - a: str = pydantic.dataclasses.Field() - b: str - c: str = pydantic.dataclasses.Field() - d: str + a: str + c: str + b: str = pydantic.dataclasses.Field() + d: str = pydantic.dataclasses.Field() b = B(a='a', b='b', c='c', d='d') for field_name in ['a', 'b', 'c', 'd']: From 0e85763c4ffd156f37ef4fc75abe0f3a29b256e5 Mon Sep 17 00:00:00 2001 From: sydney-runkle Date: Tue, 30 Jan 2024 11:25:09 -0600 Subject: [PATCH 08/12] add extra test logic --- tests/test_dataclasses.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/tests/test_dataclasses.py b/tests/test_dataclasses.py index 025d53d840..cae4dc8b25 100644 --- a/tests/test_dataclasses.py +++ b/tests/test_dataclasses.py @@ -2808,7 +2808,12 @@ class B(A): b: str = pydantic.dataclasses.Field() d: str = pydantic.dataclasses.Field() - b = B(a='a', b='b', c='c', d='d') - for field_name in ['a', 'b', 'c', 'd']: - assert B.__pydantic_fields__[field_name].annotation is str - assert getattr(b, field_name) == field_name + @pydantic.dataclasses.dataclass() + class C(B): + pass + + for _dataclass in [B, C]: + instance = _dataclass(a='a', b='b', c='c', d='d') + for field_name in ['a', 'b', 'c', 'd']: + assert _dataclass.__pydantic_fields__[field_name].annotation is str + assert getattr(instance, field_name) == field_name From 05012669dd6cfd4cd5e86c93691e1280b8ea4181 Mon Sep 17 00:00:00 2001 From: sydney-runkle Date: Tue, 30 Jan 2024 12:36:48 -0600 Subject: [PATCH 09/12] fixing tests --- pydantic/dataclasses.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/pydantic/dataclasses.py b/pydantic/dataclasses.py index a31fadff74..288001ff9c 100644 --- a/pydantic/dataclasses.py +++ b/pydantic/dataclasses.py @@ -93,7 +93,7 @@ def dataclass( @dataclass_transform(field_specifiers=(dataclasses.field, Field)) -def dataclass( # noqa: C901 +def dataclass( _cls: type[_T] | None = None, *, init: Literal[False] = False, @@ -153,6 +153,10 @@ def make_pydantic_fields_compatible(cls: type[Any]) -> None: into `x: int = dataclasses.field(default=pydantic.Field(..., kw_only=True), kw_only=True)` """ + # In Python 3.8, dataclasses checks cls.__dict__['__annotations__'] for annotations, + # so we start from there as well. + annotations = cls.__dict__.get('__annotations__') or {} + for annotation_cls in cls.__mro__: # In Python < 3.9, `__annotations__` might not be present if there are no fields. # we therefore need to use `getattr` to avoid an `AttributeError`. @@ -175,11 +179,9 @@ def make_pydantic_fields_compatible(cls: type[Any]) -> None: setattr(cls, field_name, dataclasses.field(**field_args)) - # In Python 3.8, dataclasses checks cls.__dict__['__annotations__'] for annotations, - # so we must make sure it's initialized before we add to it. - if cls.__dict__.get('__annotations__') is None: - cls.__annotations__ = {} - cls.__annotations__[field_name] = annotation_cls.__annotations__[field_name] + annotations[field_name] = annotation_cls.__annotations__[field_name] + + cls.__annotations__ = annotations def create_dataclass(cls: type[Any]) -> type[PydanticDataclass]: """Create a Pydantic dataclass from a regular dataclass. From 2d931440ab20b4c46c95ed3e533151946c13a662 Mon Sep 17 00:00:00 2001 From: sydney-runkle Date: Wed, 31 Jan 2024 08:13:52 -0600 Subject: [PATCH 10/12] modify tests to reflect modified bug --- pydantic/dataclasses.py | 9 +++++---- tests/test_dataclasses.py | 41 ++++++++++++++++++++++++++++++++------- 2 files changed, 39 insertions(+), 11 deletions(-) diff --git a/pydantic/dataclasses.py b/pydantic/dataclasses.py index 288001ff9c..ca24333f75 100644 --- a/pydantic/dataclasses.py +++ b/pydantic/dataclasses.py @@ -93,7 +93,7 @@ def dataclass( @dataclass_transform(field_specifiers=(dataclasses.field, Field)) -def dataclass( +def dataclass( # noqa: C901 _cls: type[_T] | None = None, *, init: Literal[False] = False, @@ -155,12 +155,15 @@ def make_pydantic_fields_compatible(cls: type[Any]) -> None: """ # In Python 3.8, dataclasses checks cls.__dict__['__annotations__'] for annotations, # so we start from there as well. - annotations = cls.__dict__.get('__annotations__') or {} + annotations = getattr(cls, '__annotations__', None) or cls.__dict__.get('__annotations__') or {} for annotation_cls in cls.__mro__: # In Python < 3.9, `__annotations__` might not be present if there are no fields. # we therefore need to use `getattr` to avoid an `AttributeError`. for field_name in getattr(annotation_cls, '__annotations__', []): + if annotations.get(field_name) is None: + annotations[field_name] = annotation_cls.__annotations__[field_name] + field_value = getattr(cls, field_name, None) # Process only if this is an instance of `FieldInfo`. if not isinstance(field_value, FieldInfo): @@ -179,8 +182,6 @@ def make_pydantic_fields_compatible(cls: type[Any]) -> None: setattr(cls, field_name, dataclasses.field(**field_args)) - annotations[field_name] = annotation_cls.__annotations__[field_name] - cls.__annotations__ = annotations def create_dataclass(cls: type[Any]) -> type[PydanticDataclass]: diff --git a/tests/test_dataclasses.py b/tests/test_dataclasses.py index cae4dc8b25..8b2b540343 100644 --- a/tests/test_dataclasses.py +++ b/tests/test_dataclasses.py @@ -2764,6 +2764,7 @@ class Foo: def test_annotations_valid_for_field_inheritance() -> None: # testing https://github.com/pydantic/pydantic/issues/8670 + @pydantic.dataclasses.dataclass() class A: a: int = pydantic.dataclasses.Field() @@ -2779,12 +2780,13 @@ class B(A): def test_annotations_valid_for_field_inheritance_with_existing_field() -> None: # variation on testing https://github.com/pydantic/pydantic/issues/8670 + @pydantic.dataclasses.dataclass() class A: a: int = pydantic.dataclasses.Field() @pydantic.dataclasses.dataclass() class B(A): - b: str + b: str = pydantic.dataclasses.Field() assert B.__pydantic_fields__['a'].annotation is int assert B.__pydantic_fields__['b'].annotation is str @@ -2795,12 +2797,38 @@ class B(A): def test_annotations_with_override() -> None: + @pydantic.dataclasses.dataclass() class A: a: int b: int c: int = pydantic.dataclasses.Field() d: int = pydantic.dataclasses.Field() + # note, the order of fields is different here, as to test that the annotation + # is correctly set on the field no matter the base's default / current class's default + @pydantic.dataclasses.dataclass() + class B(A): + a: str + c: str + b: str = pydantic.dataclasses.Field() + d: str = pydantic.dataclasses.Field() + + b = B(a='a', b='b', c='c', d='d') + for field_name in ['a', 'b', 'c', 'd']: + assert B.__pydantic_fields__[field_name].annotation is str + assert getattr(b, field_name) == field_name + + +def test_annotation_with_double_override() -> None: + @pydantic.dataclasses.dataclass() + class A: + a: int + b: int + c: int = pydantic.dataclasses.Field() + d: int = pydantic.dataclasses.Field() + + # note, the order of fields is different here, as to test that the annotation + # is correctly set on the field no matter the base's default / current class's default @pydantic.dataclasses.dataclass() class B(A): a: str @@ -2810,10 +2838,9 @@ class B(A): @pydantic.dataclasses.dataclass() class C(B): - pass + ... - for _dataclass in [B, C]: - instance = _dataclass(a='a', b='b', c='c', d='d') - for field_name in ['a', 'b', 'c', 'd']: - assert _dataclass.__pydantic_fields__[field_name].annotation is str - assert getattr(instance, field_name) == field_name + c = C(a='a', b='b', c='c', d='d') + for field_name in ['a', 'b', 'c', 'd']: + assert C.__pydantic_fields__[field_name].annotation is str + assert getattr(c, field_name) == field_name From 5adecc19898011960ab9cf6a8743233bc2679921 Mon Sep 17 00:00:00 2001 From: sydney-runkle Date: Wed, 31 Jan 2024 09:18:22 -0600 Subject: [PATCH 11/12] fix annotations issue + tests --- pydantic/dataclasses.py | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/pydantic/dataclasses.py b/pydantic/dataclasses.py index ca24333f75..d9c9c903b1 100644 --- a/pydantic/dataclasses.py +++ b/pydantic/dataclasses.py @@ -153,17 +153,11 @@ def make_pydantic_fields_compatible(cls: type[Any]) -> None: into `x: int = dataclasses.field(default=pydantic.Field(..., kw_only=True), kw_only=True)` """ - # In Python 3.8, dataclasses checks cls.__dict__['__annotations__'] for annotations, - # so we start from there as well. - annotations = getattr(cls, '__annotations__', None) or cls.__dict__.get('__annotations__') or {} - for annotation_cls in cls.__mro__: # In Python < 3.9, `__annotations__` might not be present if there are no fields. # we therefore need to use `getattr` to avoid an `AttributeError`. - for field_name in getattr(annotation_cls, '__annotations__', []): - if annotations.get(field_name) is None: - annotations[field_name] = annotation_cls.__annotations__[field_name] - + annotations = getattr(annotation_cls, '__annotations__', []) + for field_name in annotations: field_value = getattr(cls, field_name, None) # Process only if this is an instance of `FieldInfo`. if not isinstance(field_value, FieldInfo): @@ -181,8 +175,11 @@ def make_pydantic_fields_compatible(cls: type[Any]) -> None: field_args['repr'] = field_value.repr setattr(cls, field_name, dataclasses.field(**field_args)) - - cls.__annotations__ = annotations + # In Python 3.8, dataclasses checks cls.__dict__['__annotations__'] for annotations, + # so we must make sure it's initialized before we add to it. + if cls.__dict__.get('__annotations__') is None: + cls.__annotations__ = {} + cls.__annotations__[field_name] = annotations[field_name] def create_dataclass(cls: type[Any]) -> type[PydanticDataclass]: """Create a Pydantic dataclass from a regular dataclass. From 3f78821533247acea6ed5459f289b17de51a0bac Mon Sep 17 00:00:00 2001 From: sydney-runkle Date: Thu, 1 Feb 2024 07:48:09 -0600 Subject: [PATCH 12/12] simplifying test --- tests/test_dataclasses.py | 32 +++++--------------------------- 1 file changed, 5 insertions(+), 27 deletions(-) diff --git a/tests/test_dataclasses.py b/tests/test_dataclasses.py index 8b2b540343..dc60bd32fe 100644 --- a/tests/test_dataclasses.py +++ b/tests/test_dataclasses.py @@ -2796,29 +2796,6 @@ class B(A): assert b.b == 'b' -def test_annotations_with_override() -> None: - @pydantic.dataclasses.dataclass() - class A: - a: int - b: int - c: int = pydantic.dataclasses.Field() - d: int = pydantic.dataclasses.Field() - - # note, the order of fields is different here, as to test that the annotation - # is correctly set on the field no matter the base's default / current class's default - @pydantic.dataclasses.dataclass() - class B(A): - a: str - c: str - b: str = pydantic.dataclasses.Field() - d: str = pydantic.dataclasses.Field() - - b = B(a='a', b='b', c='c', d='d') - for field_name in ['a', 'b', 'c', 'd']: - assert B.__pydantic_fields__[field_name].annotation is str - assert getattr(b, field_name) == field_name - - def test_annotation_with_double_override() -> None: @pydantic.dataclasses.dataclass() class A: @@ -2840,7 +2817,8 @@ class B(A): class C(B): ... - c = C(a='a', b='b', c='c', d='d') - for field_name in ['a', 'b', 'c', 'd']: - assert C.__pydantic_fields__[field_name].annotation is str - assert getattr(c, field_name) == field_name + for class_ in [B, C]: + instance = class_(a='a', b='b', c='c', d='d') + for field_name in ['a', 'b', 'c', 'd']: + assert class_.__pydantic_fields__[field_name].annotation is str + assert getattr(instance, field_name) == field_name