From 623cd864453289c246c3b10ce86f68785c3d175d Mon Sep 17 00:00:00 2001 From: cody-mar10 Date: Wed, 11 Dec 2024 19:10:44 -0600 Subject: [PATCH 1/6] add better support for nested `attrs.define` style dataclasses - an edge case that is not handled is when nesting `attrs.define` dataclasses where a subfield is marked with `attrs.field` - this would typically happen when using a default factory for this field or with custom validation - the code fix checks if `attrs.asdict` is both available and appropriate to use for serializing a dataclass-like object --- jsonargparse/_signatures.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/jsonargparse/_signatures.py b/jsonargparse/_signatures.py index ee27720d..8b8afe24 100644 --- a/jsonargparse/_signatures.py +++ b/jsonargparse/_signatures.py @@ -17,12 +17,8 @@ is_subclass, ) from ._namespace import Namespace -from ._optionals import get_doc_short_description, is_pydantic_model, pydantic_support -from ._parameter_resolvers import ( - ParamData, - get_parameter_origins, - get_signature_parameters, -) +from ._optionals import attrs_support, get_doc_short_description, is_pydantic_model, pydantic_support +from ._parameter_resolvers import ParamData, get_parameter_origins, get_signature_parameters from ._typehints import ( ActionTypeHint, LazyInitBaseClass, @@ -575,6 +571,13 @@ def dataclass_to_dict(value) -> dict: pydantic_model = is_pydantic_model(type(value)) if pydantic_model: return value.dict() if pydantic_model == 1 else value.model_dump() + + if attrs_support: + import attrs + + is_attrs_dataclass = attrs.has(type(value)) + if is_attrs_dataclass: + return attrs.asdict(value) return dataclasses.asdict(value) From 72a5d1b89945ebb3bdf0b11ea22a64202d4d541a Mon Sep 17 00:00:00 2001 From: cody-mar10 Date: Thu, 12 Dec 2024 09:56:14 -0600 Subject: [PATCH 2/6] add tests for nested `attrs.define` dataclasses with and without default arguments --- jsonargparse_tests/test_dataclass_like.py | 42 +++++++++++++++++------ 1 file changed, 31 insertions(+), 11 deletions(-) diff --git a/jsonargparse_tests/test_dataclass_like.py b/jsonargparse_tests/test_dataclass_like.py index bce6865a..a970cf24 100644 --- a/jsonargparse_tests/test_dataclass_like.py +++ b/jsonargparse_tests/test_dataclass_like.py @@ -8,13 +8,7 @@ import pytest import yaml -from jsonargparse import ( - ArgumentError, - ArgumentParser, - Namespace, - compose_dataclasses, - lazy_instance, -) +from jsonargparse import ArgumentError, ArgumentParser, Namespace, compose_dataclasses, lazy_instance from jsonargparse._namespace import NSKeyError from jsonargparse._optionals import ( attrs_support, @@ -25,10 +19,7 @@ typing_extensions_import, ) from jsonargparse.typing import PositiveFloat, PositiveInt, final -from jsonargparse_tests.conftest import ( - get_parser_help, - skip_if_docstring_parser_unavailable, -) +from jsonargparse_tests.conftest import get_parser_help, skip_if_docstring_parser_unavailable annotated = typing_extensions_import("Annotated") type_alias_type = typing_extensions_import("TypeAliasType") @@ -942,6 +933,21 @@ class AttrsFieldInitFalse: def __attrs_post_init__(self): self.p1 = {} + @attrs.define + class AttrsSubField: + p1: str = "-" + p2: int = 0 + + @attrs.define + class AttrsWithNestedDefaultDataclass: + p1: float + subfield: AttrsSubField = attrs.field(factory=AttrsSubField) + + @attrs.define + class AttrsWithNestedDataclassNoDefault: + p1: float + subfield: AttrsSubField + @pytest.mark.skipif(not attrs_support, reason="attrs package is required") class TestAttrs: @@ -978,3 +984,17 @@ def test_field_init_false(self, parser): assert cfg == Namespace() init = parser.instantiate_classes(cfg) assert init.data.p1 == {} + + def test_nested_dataclass_with_default(self, parser): + # prior to changes in `jsonargparse._signatures.dataclass_to_dict` adding the nested + # attrs dataclass would lead to a TypeError with trying to use `dataclasses.asdict` + # on an attrs dataclass + parser.add_argument("--data", type=AttrsWithNestedDefaultDataclass) + cfg = parser.parse_args(["--data.p1=1.23"]) + assert cfg.data == Namespace(p1=1.23, subfield=Namespace(p1="-", p2=0)) + + def test_nested_dataclass_without_default(self, parser): + # this should have worked prior to the changes in `jsonargparse._signatures.dataclass_to_dict` + parser.add_argument("--data", type=AttrsWithNestedDefaultDataclass) + cfg = parser.parse_args(["--data.p1=1.23"]) + assert cfg.data == Namespace(p1=1.23, subfield=Namespace(p1="-", p2=0)) From 844f5b8bd1660d477a1ce7f000e8de053fcb8335 Mon Sep 17 00:00:00 2001 From: cody-mar10 Date: Thu, 12 Dec 2024 09:59:18 -0600 Subject: [PATCH 3/6] add bug fix for pull request `#641` --- CHANGELOG.rst | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 6dfdf67c..13878664 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -30,6 +30,13 @@ Deprecated Instead use ``add_class_arguments`` (`#634 `__). +v4.34.2 (2024-12-??) +-------------------- + +Fixed +^^^^^ +- Adding ``attrs.define`` dataclasses with nested dataclasses that are marked with ``attrs.field`` (such as for a default factory) are not parsed correctly (`#641 + `__) v4.34.1 (2024-12-02) -------------------- From b470ebc0bfd52c6566d752ba54808cd819e05eac Mon Sep 17 00:00:00 2001 From: Cody Martin <66684844+cody-mar10@users.noreply.github.com> Date: Thu, 12 Dec 2024 10:50:28 -0600 Subject: [PATCH 4/6] fixed pull request number to `#643` --- CHANGELOG.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 13878664..a14fc7a7 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -35,8 +35,8 @@ v4.34.2 (2024-12-??) Fixed ^^^^^ -- Adding ``attrs.define`` dataclasses with nested dataclasses that are marked with ``attrs.field`` (such as for a default factory) are not parsed correctly (`#641 - `__) +- Adding ``attrs.define`` dataclasses with nested dataclasses that are marked with ``attrs.field`` (such as for a default factory) are not parsed correctly (`#643 + `__) v4.34.1 (2024-12-02) -------------------- From 1a992c3bfae87696be90984e9f457eeb7764826b Mon Sep 17 00:00:00 2001 From: Cody Martin <66684844+cody-mar10@users.noreply.github.com> Date: Tue, 17 Dec 2024 11:42:47 -0600 Subject: [PATCH 5/6] remove comments and rename `attrs` tests for consistency additionally fixed `test_nested_without_default` to use the correct `attrs` model in the test --- jsonargparse_tests/test_dataclass_like.py | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/jsonargparse_tests/test_dataclass_like.py b/jsonargparse_tests/test_dataclass_like.py index a970cf24..4a753453 100644 --- a/jsonargparse_tests/test_dataclass_like.py +++ b/jsonargparse_tests/test_dataclass_like.py @@ -985,16 +985,12 @@ def test_field_init_false(self, parser): init = parser.instantiate_classes(cfg) assert init.data.p1 == {} - def test_nested_dataclass_with_default(self, parser): - # prior to changes in `jsonargparse._signatures.dataclass_to_dict` adding the nested - # attrs dataclass would lead to a TypeError with trying to use `dataclasses.asdict` - # on an attrs dataclass + def test_nested_with_default(self, parser): parser.add_argument("--data", type=AttrsWithNestedDefaultDataclass) cfg = parser.parse_args(["--data.p1=1.23"]) assert cfg.data == Namespace(p1=1.23, subfield=Namespace(p1="-", p2=0)) - def test_nested_dataclass_without_default(self, parser): - # this should have worked prior to the changes in `jsonargparse._signatures.dataclass_to_dict` - parser.add_argument("--data", type=AttrsWithNestedDefaultDataclass) + def test_nested_without_default(self, parser): + parser.add_argument("--data", type=AttrsWithNestedDataclassNoDefault) cfg = parser.parse_args(["--data.p1=1.23"]) assert cfg.data == Namespace(p1=1.23, subfield=Namespace(p1="-", p2=0)) From 960daf2852f6a2caf848c00241ba9cf4026ae2a3 Mon Sep 17 00:00:00 2001 From: Cody Martin <66684844+cody-mar10@users.noreply.github.com> Date: Tue, 17 Dec 2024 11:44:27 -0600 Subject: [PATCH 6/6] moved `attrs.define` dataclass fix to `v4.35.1` --- CHANGELOG.rst | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 136ccb5b..9a58560d 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -19,6 +19,9 @@ Fixed ^^^^^ - Help for ``Protocol`` types not working correctly (`#645 `__). +- Adding ``attrs.define`` dataclasses with nested dataclasses that are marked with + ``attrs.field`` (such as for a default factory) are not parsed correctly (`#643 + `__) v4.35.0 (2024-12-16) @@ -52,14 +55,6 @@ Deprecated config argument as ``--print_%s`` instead of being always ``--print_config`` (`#630 `__). -v4.34.2 (2024-12-??) --------------------- - -Fixed -^^^^^ -- Adding ``attrs.define`` dataclasses with nested dataclasses that are marked with ``attrs.field`` (such as for a default factory) are not parsed correctly (`#643 - `__) - v4.34.1 (2024-12-02) --------------------