Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nested attrs.define dataclasses not serialized correctly when a subfield is also an attrs.define dataclass marked with attrs.field #643

Merged
merged 8 commits into from
Dec 18, 2024
4 changes: 3 additions & 1 deletion CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ Fixed
^^^^^
- Help for ``Protocol`` types not working correctly (`#645
<https://github.com/omni-us/jsonargparse/pull/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
<https://github.com/omni-us/jsonargparse/pull/643>`__)


v4.35.0 (2024-12-16)
Expand Down Expand Up @@ -52,7 +55,6 @@ Deprecated
config argument as ``--print_%s`` instead of being always ``--print_config``
(`#630 <https://github.com/omni-us/jsonargparse/pull/630>`__).


v4.34.1 (2024-12-02)
--------------------

Expand Down
15 changes: 9 additions & 6 deletions jsonargparse/_signatures.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)


Expand Down
38 changes: 27 additions & 11 deletions jsonargparse_tests/test_dataclass_like.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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")
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -978,3 +984,13 @@ def test_field_init_false(self, parser):
assert cfg == Namespace()
init = parser.instantiate_classes(cfg)
assert init.data.p1 == {}

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_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))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks somewhat unexpected. I would have guessed that the parsed value would be Namespace(p1=1.23, subfield=None). But anyway, this is not related to this pull request so I would say to keep it like this for now. Probably this behavior will need to change for #287.

Loading