From b2d0716ed11f4ad9283e8acb856d77be5e559336 Mon Sep 17 00:00:00 2001 From: Vineeth Sai Date: Sun, 5 Jul 2026 23:31:17 -0700 Subject: [PATCH] Disambiguate unions independent of member class order create_default_dis_func resolved unique discriminator fields in a single fixed-order pass: for each class it subtracted the fields of every not-yet-pinned class, so a class whose fields were all shared with another unpinned class got no unique field and was forced into the single fallback slot. A second such class then raised TypeError, even when a valid assignment existed. Because the order was fixed up front, whether it was found depended on the order the classes were passed in. Resolve unique fields iteratively to a fixpoint instead: on each pass, pin every class that has a non-default field unique among the classes not yet pinned; pinning a class can make a previously ambiguous one uniquely identifiable, so repeat until a pass pins nothing. The single class that still cannot be pinned becomes the fallback, and more than one is a genuine ambiguity (still a TypeError). Fixes #230. --- HISTORY.md | 2 ++ src/cattrs/disambiguators.py | 68 ++++++++++++++++++++++-------------- tests/test_disambiguators.py | 34 ++++++++++++++++++ 3 files changed, 77 insertions(+), 27 deletions(-) diff --git a/HISTORY.md b/HISTORY.md index 2942ad33..07b1c970 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -13,6 +13,8 @@ Our backwards-compatibility policy can be found [here](https://github.com/python ## NEXT (UNRELEASED) +- Fix `create_default_dis_func ` (aka `create_uniq_field_dis_func`) failing to disambiguate valid unions depending on the order of the member classes; unique fields are now resolved iteratively to a fixpoint. + ([#230](https://github.com/python-attrs/cattrs/issues/230) [#765](https://github.com/python-attrs/cattrs/pull/765)) - Support more recursive types on 3.14+ with specialized factories for [`annotationlib.ForwardRef`](https://docs.python.org/3/library/annotationlib.html#annotationlib.ForwardRef). ([#740](https://github.com/python-attrs/cattrs/issues/740) [#741](https://github.com/python-attrs/cattrs/pull/741)) - `Converter ` now uses specialized hook factories to generate hooks for tuples, increasing speed. diff --git a/src/cattrs/disambiguators.py b/src/cattrs/disambiguators.py index f6e43924..f020c844 100644 --- a/src/cattrs/disambiguators.py +++ b/src/cattrs/disambiguators.py @@ -131,33 +131,47 @@ def dis_func(data: Mapping[Any, Any]) -> type | None: # We start from classes with the largest number of unique fields # so we can do easy picks first, making later picks easier. - cls_and_attrs.sort(key=lambda c_a: len(c_a[1]), reverse=True) - - fallback = None # If none match, try this. - - for cl, cl_reqs, back_map in cls_and_attrs: - # We do not have to consider classes we've already processed, since - # they will have been eliminated by the match dictionary already. - other_classes = [ - c_and_a - for c_and_a in cls_and_attrs - if c_and_a[0] is not cl and c_and_a[0] not in uniq_attrs_dict.values() - ] - other_reqs = reduce(or_, (c_a[1] for c_a in other_classes), 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): - break - else: - if fallback is None: - fallback = cl - continue - raise TypeError(f"{cl} has no usable non-default attributes") - uniq_attrs_dict[maybe_renamed_attr_name] = cl + remaining = sorted(cls_and_attrs, key=lambda c_a: len(c_a[1]), reverse=True) + + # 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 + + # 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. + if len(remaining) > 1: + raise TypeError( + "Could not disambiguate between " + + ", ".join(repr(c_a[0]) for c_a in remaining) + + ": no unique non-default attributes" + ) + fallback = remaining[0][0] if remaining else None if fallback is None: diff --git a/tests/test_disambiguators.py b/tests/test_disambiguators.py index bee89a12..2ae5090f 100644 --- a/tests/test_disambiguators.py +++ b/tests/test_disambiguators.py @@ -2,6 +2,7 @@ from dataclasses import dataclass from functools import partial +from itertools import permutations from typing import Literal, Union import pytest @@ -113,6 +114,39 @@ class L: fn([]) +def test_disambiguation_is_order_independent(): + """A valid union disambiguates regardless of the order of its members. + + Regression test for #230: a class whose fields were all shared with another, + not-yet-pinned class was forced into the single fallback slot, so some + member orderings raised ``TypeError`` even though a valid assignment exists. + Here A is identified by ``a``, B by ``ab`` (once A is pinned), and C by + ``bc`` (once B is pinned); a single fixed-order pass misses those later + picks for half of the orderings. + """ + c = Converter() + + @define + class A: + ab: int + a: str + + @define + class B: + ab: int + bc: int + + @define + class C: + bc: int + + for ordering in permutations((A, B, C)): + fn = create_default_dis_func(c, *ordering) + assert fn(asdict(A(1, "x"))) is A + assert fn(asdict(B(1, 2))) is B + assert fn(asdict(C(1))) is C + + def test_input_not_mapping(): """Correct errors are raised when the raw payload isn't a mapping.""" c = Converter()