From 332e8cc8635b8a48796b84bcc805ca12db5b94b1 Mon Sep 17 00:00:00 2001 From: Antony Frolov Date: Tue, 4 Nov 2025 22:21:58 +0000 Subject: [PATCH 1/5] Apply attrs' converter to default before omit_if_default check --- src/cattrs/gen/__init__.py | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/src/cattrs/gen/__init__.py b/src/cattrs/gen/__init__.py index 62e3a37a..12f0999b 100644 --- a/src/cattrs/gen/__init__.py +++ b/src/cattrs/gen/__init__.py @@ -4,7 +4,7 @@ from collections.abc import Callable, Iterable, Mapping from typing import TYPE_CHECKING, Any, Final, Literal, TypeVar -from attrs import NOTHING, Attribute, Factory +from attrs import NOTHING, Attribute, Factory, Converter from typing_extensions import NoDefault from .._compat import ( @@ -178,15 +178,26 @@ def make_dict_unstructure_fn_from_attrs( globs[def_name] = d.factory internal_arg_parts[def_name] = d.factory if d.takes_self: - lines.append(f" if instance.{attr_name} != {def_name}(instance):") + def_str = f"{def_name}(instance)" else: - lines.append(f" if instance.{attr_name} != {def_name}():") - lines.append(f" res['{kn}'] = {invoke}") + def_str = f"{def_name}()" else: globs[def_name] = d internal_arg_parts[def_name] = d - lines.append(f" if instance.{attr_name} != {def_name}:") - lines.append(f" res['{kn}'] = {invoke}") + def_str = def_name + + c = a.converter + if isinstance(c, Converter): + conv_name = f"__c_conv_{attr_name}" + globs[conv_name] = c + internal_arg_parts[conv_name] = c + field_name = f"__c_field_{attr_name}" + globs[field_name] = a + internal_arg_parts[field_name] = a + def_str = f"{conv_name}({def_str}, instance, {field_name})" + + lines.append(f" if instance.{attr_name} != {def_str}:") + lines.append(f" res['{kn}'] = {invoke}") else: # No default or no override. From af4b5a069ae80119be69874bb1434d30952f7aa0 Mon Sep 17 00:00:00 2001 From: Antony Frolov Date: Wed, 5 Nov 2025 09:25:23 +0000 Subject: [PATCH 2/5] Fix ruff --- src/cattrs/gen/__init__.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/cattrs/gen/__init__.py b/src/cattrs/gen/__init__.py index 12f0999b..2a408a05 100644 --- a/src/cattrs/gen/__init__.py +++ b/src/cattrs/gen/__init__.py @@ -4,7 +4,7 @@ from collections.abc import Callable, Iterable, Mapping from typing import TYPE_CHECKING, Any, Final, Literal, TypeVar -from attrs import NOTHING, Attribute, Factory, Converter +from attrs import NOTHING, Attribute, Converter, Factory from typing_extensions import NoDefault from .._compat import ( @@ -177,10 +177,7 @@ def make_dict_unstructure_fn_from_attrs( if isinstance(d, Factory): globs[def_name] = d.factory internal_arg_parts[def_name] = d.factory - if d.takes_self: - def_str = f"{def_name}(instance)" - else: - def_str = f"{def_name}()" + def_str = f"{def_name}(instance)" if d.takes_self else f"{def_name}()" else: globs[def_name] = d internal_arg_parts[def_name] = d From 0a8358b4a3181882747ff7a0bc23f0096ce68477 Mon Sep 17 00:00:00 2001 From: Anton Frolov Date: Sun, 9 Nov 2025 13:50:18 +0000 Subject: [PATCH 3/5] Add a test --- src/cattrs/gen/__init__.py | 13 ++++++++----- tests/test_converter.py | 26 ++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 5 deletions(-) diff --git a/src/cattrs/gen/__init__.py b/src/cattrs/gen/__init__.py index 2a408a05..41d3c6e3 100644 --- a/src/cattrs/gen/__init__.py +++ b/src/cattrs/gen/__init__.py @@ -184,14 +184,17 @@ def make_dict_unstructure_fn_from_attrs( def_str = def_name c = a.converter - if isinstance(c, Converter): + if c is not None: conv_name = f"__c_conv_{attr_name}" globs[conv_name] = c internal_arg_parts[conv_name] = c - field_name = f"__c_field_{attr_name}" - globs[field_name] = a - internal_arg_parts[field_name] = a - def_str = f"{conv_name}({def_str}, instance, {field_name})" + if isinstance(c, Converter): + field_name = f"__c_field_{attr_name}" + globs[field_name] = a + internal_arg_parts[field_name] = a + def_str = f"{conv_name}({def_str}, instance, {field_name})" + else: + def_str = f"{conv_name}({def_str})" lines.append(f" if instance.{attr_name} != {def_str}:") lines.append(f" res['{kn}'] = {invoke}") diff --git a/tests/test_converter.py b/tests/test_converter.py index 3c5cbe25..d2b7f6a1 100644 --- a/tests/test_converter.py +++ b/tests/test_converter.py @@ -16,6 +16,7 @@ Union, ) +import attrs import pytest from attrs import Factory, define, field, fields, has, make_class from hypothesis import assume, given @@ -339,6 +340,31 @@ class C: assert inst == converter.structure(unstructured, C) +@given(simple_typed_classes(defaults="always", allow_nan=False)) +def test_omit_default_with_attrs_converter_roundtrip(cl_and_vals): + """ + Omit default with attrs' converter on the converter works. + """ + converter = Converter(omit_if_default=True) + cl, vals, kwargs = cl_and_vals + + @define + class C: + a1: int = field(default="1", converter=int) + a2: int = field(default="1", converter=attrs.Converter(int)) + c: cl = Factory(lambda: cl(*vals, **kwargs)) + + inst = C() + unstructured = converter.unstructure(inst) + assert unstructured == {} + assert inst == converter.structure(unstructured, C) + + inst = C(0, 0) + unstructured = converter.unstructure(inst) + assert unstructured == {"a1": 0, "a2": 0} + assert inst == converter.structure(unstructured, C) + + def test_dict_roundtrip_with_alias(): """ A class with an aliased attribute can be unstructured and structured. From 005cd386262609d8166de02168ba6f6f33e0316a Mon Sep 17 00:00:00 2001 From: Anton Frolov Date: Sun, 23 Nov 2025 20:43:51 +0000 Subject: [PATCH 4/5] Add docs --- HISTORY.md | 2 ++ src/cattrs/gen/__init__.py | 15 ++++++++++++--- tests/test_converter.py | 8 +++++--- 3 files changed, 19 insertions(+), 6 deletions(-) diff --git a/HISTORY.md b/HISTORY.md index 4339550f..e1b5298d 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -17,6 +17,8 @@ Our backwards-compatibility policy can be found [here](https://github.com/python ([#688](https://github.com/python-attrs/cattrs/pull/688)) - Python 3.9 is no longer supported, as it is end-of-life. Use previous versions on this Python version. ([#698](https://github.com/python-attrs/cattrs/pull/698)) +- Apply the attrs converter to the default value before checking if it is equal to the attribute's value, when `omit_if_default` is true and an attrs converter is specified. + ([#696](https://github.com/python-attrs/cattrs/pull/696)) ## 25.3.0 (2025-10-07) diff --git a/src/cattrs/gen/__init__.py b/src/cattrs/gen/__init__.py index 41d3c6e3..3357aaca 100644 --- a/src/cattrs/gen/__init__.py +++ b/src/cattrs/gen/__init__.py @@ -99,6 +99,10 @@ def make_dict_unstructure_fn_from_attrs( .. versionchanged:: 25.2.0 The `_cattrs_use_alias` parameter takes its value from the given converter by default. + .. versionchanged:: NEXT + When `_cattrs_omit_if_default` is true and the attribute has an attrs converter + specified, the converter is applied to the default value before checking if it + is equal to the attribute's value. """ fn_name = "unstructure_" + cl.__name__ @@ -186,15 +190,20 @@ def make_dict_unstructure_fn_from_attrs( c = a.converter if c is not None: conv_name = f"__c_conv_{attr_name}" - globs[conv_name] = c - internal_arg_parts[conv_name] = c if isinstance(c, Converter): + globs[conv_name] = c + internal_arg_parts[conv_name] = c field_name = f"__c_field_{attr_name}" globs[field_name] = a internal_arg_parts[field_name] = a def_str = f"{conv_name}({def_str}, instance, {field_name})" - else: + elif isinstance(d, Factory): + globs[conv_name] = c + internal_arg_parts[conv_name] = c def_str = f"{conv_name}({def_str})" + else: + globs[def_name] = c(d) + internal_arg_parts[def_name] = c(d) lines.append(f" if instance.{attr_name} != {def_str}:") lines.append(f" res['{kn}'] = {invoke}") diff --git a/tests/test_converter.py b/tests/test_converter.py index d2b7f6a1..a137be55 100644 --- a/tests/test_converter.py +++ b/tests/test_converter.py @@ -343,7 +343,7 @@ class C: @given(simple_typed_classes(defaults="always", allow_nan=False)) def test_omit_default_with_attrs_converter_roundtrip(cl_and_vals): """ - Omit default with attrs' converter on the converter works. + Omit default with an attrs converter works. """ converter = Converter(omit_if_default=True) cl, vals, kwargs = cl_and_vals @@ -352,6 +352,8 @@ def test_omit_default_with_attrs_converter_roundtrip(cl_and_vals): class C: a1: int = field(default="1", converter=int) a2: int = field(default="1", converter=attrs.Converter(int)) + a3: int = field(factory=lambda: "1", converter=int) + a4: int = field(factory=lambda: "1", converter=attrs.Converter(int)) c: cl = Factory(lambda: cl(*vals, **kwargs)) inst = C() @@ -359,9 +361,9 @@ class C: assert unstructured == {} assert inst == converter.structure(unstructured, C) - inst = C(0, 0) + inst = C(0, 0, 0, 0) unstructured = converter.unstructure(inst) - assert unstructured == {"a1": 0, "a2": 0} + assert unstructured == {"a1": 0, "a2": 0, "a3": 0, "a4": 0} assert inst == converter.structure(unstructured, C) From 50b472a4b09d6e16d893a394bff1a2f75473f323 Mon Sep 17 00:00:00 2001 From: Anton Frolov Date: Fri, 28 Nov 2025 19:15:00 +0000 Subject: [PATCH 5/5] Add more attrs to test --- tests/test_converter.py | 35 +++++++++++++++++++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-) diff --git a/tests/test_converter.py b/tests/test_converter.py index a137be55..ab86d55e 100644 --- a/tests/test_converter.py +++ b/tests/test_converter.py @@ -354,6 +354,29 @@ class C: a2: int = field(default="1", converter=attrs.Converter(int)) a3: int = field(factory=lambda: "1", converter=int) a4: int = field(factory=lambda: "1", converter=attrs.Converter(int)) + a5: int = field( + factory=lambda: "2", + converter=attrs.Converter( + lambda obj, self: int(obj) + self.a4, takes_self=True + ), + ) + a6: int = field( + factory=lambda: "2", + converter=attrs.Converter( + lambda obj, field: int(obj) + int(field.default.factory()) - 2, + takes_field=True, + ), + ) + a7: int = field( + factory=lambda: "3", + converter=attrs.Converter( + lambda obj, self, field: ( + int(obj) + self.a6 + int(field.default.factory()) - 3 + ), + takes_self=True, + takes_field=True, + ), + ) c: cl = Factory(lambda: cl(*vals, **kwargs)) inst = C() @@ -361,9 +384,17 @@ class C: assert unstructured == {} assert inst == converter.structure(unstructured, C) - inst = C(0, 0, 0, 0) + inst = C(0, 0, 0, 0, 0, 0, 0) unstructured = converter.unstructure(inst) - assert unstructured == {"a1": 0, "a2": 0, "a3": 0, "a4": 0} + assert unstructured == { + "a1": 0, + "a2": 0, + "a3": 0, + "a4": 0, + "a5": 0, + "a6": 0, + "a7": 0, + } assert inst == converter.structure(unstructured, C)