diff --git a/HISTORY.md b/HISTORY.md index 942616e3..917c7922 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -13,6 +13,8 @@ Our backwards-compatibility policy can be found [here](https://github.com/python ## NEXT +- The default union disambiguator now uses unique fields with defaults as a fallback signal when not enough classes can be told apart by unique fields without defaults, so unions of classes whose fields all carry defaults (for example two dataclasses with a differently-named field each) can be structured instead of failing with "no unique non-default attributes". + ([#521](https://github.com/python-attrs/cattrs/issues/521)) - Fix the msgspec converter passing typed deques directly to the encoder, which cannot serialize them. Deques are now unstructured into lists, including when nested in attrs classes or dataclasses. ([#818](https://github.com/python-attrs/cattrs/issues/818) [#819](https://github.com/python-attrs/cattrs/pull/819)) diff --git a/src/cattrs/disambiguators.py b/src/cattrs/disambiguators.py index 451d5768..5049cb8c 100644 --- a/src/cattrs/disambiguators.py +++ b/src/cattrs/disambiguators.py @@ -55,6 +55,9 @@ def create_default_dis_func( Dataclasses are now supported. .. versionchanged:: NEXT Renaming overrides also apply to literal discriminator fields. + .. versionchanged:: NEXT + Unique fields with defaults are now used as a fallback signal when + not enough classes can be pinned by unique fields without defaults. """ if len(classes) < 2: raise ValueError("At least two classes required.") @@ -126,45 +129,24 @@ def dis_func(data: Mapping[Any, Any]) -> type | None: (cl, *_usable_attribute_names(cl, override)) for cl, override in zip(classes, overrides) ] - # For each class, attempt to generate a single unique required field. uniq_attrs_dict: dict[str, type] = {} # We start from classes with the largest number of unique fields # so we can do easy picks first, making later picks easier. remaining = sorted(cls_and_attrs, key=lambda c_a: len(c_a[1]), reverse=True) + remaining = _pin_unique_fields(remaining, uniq_attrs_dict, defaulted=False) - # Iterate to a fixpoint. On each pass, pin every class that has a - # non-default field unique among the classes not yet pinned. Pinning a - # class removes its fields from consideration, which can make a previously - # ambiguous class uniquely identifiable, so we repeat until a pass pins - # nothing. A single pass in a fixed order would miss those later picks and - # fail to disambiguate valid unions depending on the class order. - made_progress = True - while made_progress: - made_progress = False - still_remaining = [] - for entry in remaining: - cl, cl_reqs, back_map = entry - other_reqs = reduce( - or_, (c_a[1] for c_a in remaining if c_a[0] is not cl), set() - ) - uniq = cl_reqs - other_reqs - - # We want a unique attribute with no default. - cl_fields = fields_dict(get_origin(cl) or cl) - for maybe_renamed_attr_name in uniq: - orig_name = back_map[maybe_renamed_attr_name] - if cl_fields[orig_name].default in (NOTHING, MISSING): - uniq_attrs_dict[maybe_renamed_attr_name] = cl - made_progress = True - break - else: - still_remaining.append(entry) - remaining = still_remaining + # Classes whose distinguishing fields all carry defaults cannot be pinned + # by the pass above: a defaulted field may be absent from the payload, so + # it is only a signal when present. If more than one class is still + # unpinned, give those fields a chance too; cases that disambiguate today + # are unaffected, since this pass only runs where the first one failed. + if len(remaining) > 1: + remaining = _pin_unique_fields(remaining, uniq_attrs_dict, defaulted=True) - # A class we could not pin to a unique non-default field can only be the - # single fallback (tried when no unique key matches). More than one such - # class is a genuine ambiguity. + # A class we could not pin to a unique field can only be the single + # fallback (tried when no unique key matches). More than one such class + # is a genuine ambiguity. if len(remaining) > 1: raise TypeError( "Could not disambiguate between " @@ -199,6 +181,45 @@ def dis_func(data: Mapping[Any, Any]) -> type[AttrsInstance] | None: create_uniq_field_dis_func = create_default_dis_func +def _pin_unique_fields( + remaining: list, uniq_attrs_dict: dict[str, type], *, defaulted: bool +) -> list: + """Pin classes to a field unique among the remaining ones, to a fixpoint. + + With `defaulted=False` only fields without a default qualify; with + `defaulted=True` only fields with one do. Pinning a class removes its + fields from consideration, which can make a previously ambiguous class + uniquely identifiable, so passes repeat until one pins nothing. A single + pass in a fixed order would miss those later picks and fail to + disambiguate valid unions depending on the class order. + + Return the classes that could not be pinned. + """ + made_progress = True + while made_progress: + made_progress = False + still_remaining = [] + for entry in remaining: + cl, cl_reqs, back_map = entry + other_reqs = reduce( + or_, (c_a[1] for c_a in remaining if c_a[0] is not cl), set() + ) + uniq = cl_reqs - other_reqs + + cl_fields = fields_dict(get_origin(cl) or cl) + for maybe_renamed_attr_name in uniq: + orig_name = back_map[maybe_renamed_attr_name] + has_default = cl_fields[orig_name].default not in (NOTHING, MISSING) + if has_default == defaulted: + uniq_attrs_dict[maybe_renamed_attr_name] = cl + made_progress = True + break + else: + still_remaining.append(entry) + remaining = still_remaining + return remaining + + def _overriden_name(at: Attribute, override: AttributeOverride | None) -> str: if override is None or override.rename is None: return at.name diff --git a/tests/test_disambiguators.py b/tests/test_disambiguators.py index 66f366d8..efd950d1 100644 --- a/tests/test_disambiguators.py +++ b/tests/test_disambiguators.py @@ -495,3 +495,67 @@ class B: assert converter.structure({"a": "a"}, Union[A, B]) == A() assert converter.structure({"b": "b"}, Union[A, B]) == B("b") + + +def test_defaulted_unique_fields(converter: Converter): + """Classes told apart only by defaulted unique fields still disambiguate. + + A defaulted field is a signal when present in the payload. + """ + + @define + class A: + a: str = "" + + @define + class B: + b: str = "" + + dis_func = create_default_dis_func(converter, A, B) + + assert dis_func({"a": "x"}) is A + assert dis_func({"b": "x"}) is B + with pytest.raises(ValueError): + # Neither signal present; both classes would accept the payload. + dis_func({}) + + assert converter.structure({"a": "x"}, Union[A, B]) == A("x") + assert converter.structure({"b": "x"}, Union[A, B]) == B("x") + + +def test_defaulted_unique_fields_chain(converter: Converter): + """A defaulted field can become unique only after another class is pinned.""" + + @define + class A: + shared: str = "" + only_a: str = "" + + @define + class B: + shared: str = "" + + @define + class C: + only_c: int + + dis_func = create_default_dis_func(converter, A, B, C) + + assert dis_func({"only_c": 1}) is C + assert dis_func({"only_a": "x"}) is A + assert dis_func({"shared": "x"}) is B + + +def test_defaulted_fields_do_not_unblock_true_ambiguity(converter: Converter): + """Identically-shaped classes still cannot be disambiguated.""" + + @define + class A: + a: str = "" + + @define + class B: + a: str = "" + + with pytest.raises(TypeError): + create_default_dis_func(converter, A, B)