From 539908a9754d79c3ba0735639a44c62157d9c293 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damian=20=C5=9Awistowski?= Date: Fri, 13 Nov 2020 12:22:28 +0000 Subject: [PATCH 1/6] fix structure for union of attr classes with default values --- src/cattr/disambiguators.py | 11 +++++++++-- tests/test_structure.py | 19 +++++++++++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/src/cattr/disambiguators.py b/src/cattr/disambiguators.py index 2c472191..ada129f7 100644 --- a/src/cattr/disambiguators.py +++ b/src/cattr/disambiguators.py @@ -10,7 +10,7 @@ Type, ) -from attr import fields +from attr import fields, NOTHING from cattr._compat import get_origin @@ -22,7 +22,14 @@ def create_uniq_field_dis_func(*classes: Type) -> Callable: if len(classes) < 2: raise ValueError("At least two classes required.") cls_and_attrs = [ - (cl, set(at.name for at in fields(get_origin(cl) or cl))) + ( + cl, + set( + at.name + for at in fields(get_origin(cl) or cl) + if at.default is NOTHING + ), + ) for cl in classes ] if len([attrs for _, attrs in cls_and_attrs if len(attrs) == 0]) > 1: diff --git a/tests/test_structure.py b/tests/test_structure.py index b41957cc..bbdde276 100644 --- a/tests/test_structure.py +++ b/tests/test_structure.py @@ -29,6 +29,7 @@ ) from pytest import raises +import attr from cattr import Converter from cattr._compat import is_bare, is_union_type from cattr.converters import NoneType @@ -351,3 +352,21 @@ class Bar(Foo): converter.register_structure_hook(Bar, lambda obj, cls: cls("bar")) assert converter.structure(None, Foo).value == "foo" assert converter.structure(None, Bar).value == "bar" + + +def test_structure_union_edge_case(): + converter = Converter() + + @attr.s(auto_attribs=True) + class A: + a1: Any + a2: Optional[Any] = None + + @attr.s(auto_attribs=True) + class B: + b1: Any + b2: Optional[Any] = None + + assert converter.structure( + [{"a1": "foo"}, {"b1": "bar"}], List[Union[A, B]] + ) == [A("foo"), B("bar")] From a18340dca7b1a00630cfdb175bb70319c8640a23 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damian=20=C5=9Awistowski?= Date: Mon, 16 Nov 2020 11:24:15 +0000 Subject: [PATCH 2/6] apply Tinche suggestion from PR --- src/cattr/disambiguators.py | 9 ++++++++- tests/test_disambigutors.py | 12 ++++++++++-- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/src/cattr/disambiguators.py b/src/cattr/disambiguators.py index ada129f7..fbbd902a 100644 --- a/src/cattr/disambiguators.py +++ b/src/cattr/disambiguators.py @@ -49,7 +49,14 @@ def create_uniq_field_dis_func(*classes: Type) -> Callable: if not uniq: m = "{} has no usable unique attributes.".format(cl) raise ValueError(m) - uniq_attrs_dict[next(iter(uniq))] = cl + # We need a unique attribute with no default. + cl_fields = fields(get_origin(cl) or cl) + for attr_name in uniq: + if getattr(cl_fields, attr_name).default is NOTHING: + break + else: + raise Exception(f"{cl} has no usable non-default attributes.") + uniq_attrs_dict[attr_name] = cl else: fallback = cl diff --git a/tests/test_disambigutors.py b/tests/test_disambigutors.py index c9078e7b..3df827e3 100644 --- a/tests/test_disambigutors.py +++ b/tests/test_disambigutors.py @@ -2,7 +2,8 @@ import attr import pytest -from hypothesis import assume, given +from attr import NOTHING +from hypothesis import HealthCheck, assume, given, settings from cattr.disambiguators import create_uniq_field_dis_func @@ -63,9 +64,12 @@ class A(object): fn({"xyz": 1}) is A # Uses the fallback. +@settings( + suppress_health_check=[HealthCheck.filter_too_much, HealthCheck.too_slow] +) @given(simple_classes(), simple_classes()) def test_disambiguation(cl_and_vals_a, cl_and_vals_b): - """Disambiguation should work when there are unique fields.""" + """Disambiguation should work when there are unique required fields.""" cl_a, vals_a = cl_and_vals_a cl_b, vals_b = cl_and_vals_b @@ -76,6 +80,10 @@ def test_disambiguation(cl_and_vals_a, cl_and_vals_b): assume(len(req_b)) assume((req_a - req_b) or (req_b - req_a)) + for attr_name in req_a - req_b: + assume(getattr(attr.fields(cl_a), attr_name).default is NOTHING) + for attr_name in req_b - req_a: + assume(getattr(attr.fields(cl_b), attr_name).default is NOTHING) fn = create_uniq_field_dis_func(cl_a, cl_b) From 20557b33f1a27573673123c47bc70320ff51b8ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damian=20=C5=9Awistowski?= Date: Mon, 16 Nov 2020 11:30:08 +0000 Subject: [PATCH 3/6] update history --- HISTORY.rst | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/HISTORY.rst b/HISTORY.rst index 78893c50..8dde8be5 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -2,6 +2,11 @@ History ======= +1.1.2 (unrelased) +----------------- +* Disambigualation do not pick non required field. + (`#108 `_) + 1.1.1 (2020-10-30) ------------------ * Add metadata for supported Python versions. From 889ff6229a53fbad79a277e2c2aa32134f0fc363 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damian=20=C5=9Awistowski?= Date: Mon, 16 Nov 2020 15:37:21 +0000 Subject: [PATCH 4/6] remove forgoten code --- src/cattr/disambiguators.py | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/src/cattr/disambiguators.py b/src/cattr/disambiguators.py index fbbd902a..9b69715b 100644 --- a/src/cattr/disambiguators.py +++ b/src/cattr/disambiguators.py @@ -22,15 +22,7 @@ def create_uniq_field_dis_func(*classes: Type) -> Callable: if len(classes) < 2: raise ValueError("At least two classes required.") cls_and_attrs = [ - ( - cl, - set( - at.name - for at in fields(get_origin(cl) or cl) - if at.default is NOTHING - ), - ) - for cl in classes + (cl, set(at.name for at in fields(get_origin(cl) or cl))) for cl in classes ] if len([attrs for _, attrs in cls_and_attrs if len(attrs) == 0]) > 1: raise ValueError("At least two classes have no attributes.") From 67ac83409e9289e480d9cc6360a2aeecc74ed0ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damian=20=C5=9Awistowski?= Date: Mon, 16 Nov 2020 15:42:36 +0000 Subject: [PATCH 5/6] apply black --- src/cattr/disambiguators.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/cattr/disambiguators.py b/src/cattr/disambiguators.py index 9b69715b..e10908fb 100644 --- a/src/cattr/disambiguators.py +++ b/src/cattr/disambiguators.py @@ -22,7 +22,8 @@ def create_uniq_field_dis_func(*classes: Type) -> Callable: if len(classes) < 2: raise ValueError("At least two classes required.") cls_and_attrs = [ - (cl, set(at.name for at in fields(get_origin(cl) or cl))) for cl in classes + (cl, set(at.name for at in fields(get_origin(cl) or cl))) + for cl in classes ] if len([attrs for _, attrs in cls_and_attrs if len(attrs) == 0]) > 1: raise ValueError("At least two classes have no attributes.") From ba9cd71530783974f98bc0948d1641a280a4cb8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damian=20=C5=9Awistowski?= Date: Mon, 16 Nov 2020 16:32:52 +0000 Subject: [PATCH 6/6] add test for no usable non-default attributes --- src/cattr/disambiguators.py | 2 +- tests/test_disambigutors.py | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/cattr/disambiguators.py b/src/cattr/disambiguators.py index e10908fb..ecc2e233 100644 --- a/src/cattr/disambiguators.py +++ b/src/cattr/disambiguators.py @@ -48,7 +48,7 @@ def create_uniq_field_dis_func(*classes: Type) -> Callable: if getattr(cl_fields, attr_name).default is NOTHING: break else: - raise Exception(f"{cl} has no usable non-default attributes.") + raise ValueError(f"{cl} has no usable non-default attributes.") uniq_attrs_dict[attr_name] = cl else: fallback = cl diff --git a/tests/test_disambigutors.py b/tests/test_disambigutors.py index 3df827e3..4da1ca14 100644 --- a/tests/test_disambigutors.py +++ b/tests/test_disambigutors.py @@ -1,4 +1,6 @@ """Tests for auto-disambiguators.""" +from typing import Any + import attr import pytest @@ -41,6 +43,18 @@ class D(object): # No unique fields on either class. create_uniq_field_dis_func(C, D) + @attr.s + class E: + pass + + @attr.s + class F: + b = attr.ib(default=Any) + + with pytest.raises(ValueError): + # no usable non-default attributes + create_uniq_field_dis_func(E, F) + @given(simple_classes(defaults=False)) def test_fallback(cl_and_vals):