From 8d869704ff9492ed651f8ad065eb6f656f58565b Mon Sep 17 00:00:00 2001 From: Matthieu Melot Date: Sun, 6 Nov 2022 23:03:17 -0500 Subject: [PATCH 01/17] make.bat mirror update --- docs/make.bat | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/docs/make.bat b/docs/make.bat index dc68b6e6..a48f35b6 100644 --- a/docs/make.bat +++ b/docs/make.bat @@ -239,4 +239,11 @@ if "%1" == "pseudoxml" ( goto end ) +if "%1" == "apidoc" ( + sphinx-apidoc -o . ../src/cattrs/ -f + if errorlevel 1 exit /b 1 + echo. + goto end +) + :end From f2b14465791a0e2d3c427ca64887ac7cae63c0dd Mon Sep 17 00:00:00 2001 From: Matthieu Melot Date: Wed, 2 Nov 2022 22:50:58 -0400 Subject: [PATCH 02/17] subclasses with strategies as proposed by Tin. --- src/cattrs/strategies/__init__.py | 3 +- src/cattrs/strategies/_subclasses.py | 53 ++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 1 deletion(-) create mode 100644 src/cattrs/strategies/_subclasses.py diff --git a/src/cattrs/strategies/__init__.py b/src/cattrs/strategies/__init__.py index af95dd96..0dfc39a2 100644 --- a/src/cattrs/strategies/__init__.py +++ b/src/cattrs/strategies/__init__.py @@ -1,4 +1,5 @@ """High level strategies for converters.""" +from ._subclasses import include_subclasses from ._unions import configure_tagged_union -__all__ = ["configure_tagged_union"] +__all__ = ["configure_tagged_union", "include_subclasses"] diff --git a/src/cattrs/strategies/_subclasses.py b/src/cattrs/strategies/_subclasses.py new file mode 100644 index 00000000..06837502 --- /dev/null +++ b/src/cattrs/strategies/_subclasses.py @@ -0,0 +1,53 @@ +"""Strategies for customizing subclass behaviors.""" +from gc import collect +from typing import Dict, Optional, Tuple, Type, Union, List + +from ..converters import Converter + +def include_subclasses( + cl: Type, converter: Converter, subclasses: Optional[Tuple[Type]] = None +) -> None: + """ + Modify the given converter so that the attrs/dataclass `cl` is + un/structured as if it was a union of itself and all its subclasses + that are defined at the time when this strategy is applied. + + Subclasses are detected using the `__subclasses__` method, or + they can be explicitly provided. + """ + # Due to https://github.com/python-attrs/attrs/issues/1047 + collect() + # This hook is for instances of A, but not instances of subclasses. + base_hook = converter.gen_unstructure_attrs_fromdict(cl) + + def unstructure_a(val: cl, c=converter) -> Dict: + """ + If val is an instance of `A`, use the hook. + + If val is an instance of a subclass, dispatch on its exact + runtime type. + """ + if val.__class__ is cl: + return base_hook(val) + return c.unstructure(val, unstructure_as=val.__class__) + + # This needs to use function dispatch, using singledispatch will again + # match A and all subclasses, which is not what we want. + converter.register_unstructure_hook_func(lambda cls: cls is cl, unstructure_a) + + if subclasses is not None: + subclass_union = Union[(cl, subclasses)] + else: + subclass_union = Union[(cl, *cl.__subclasses__())] + + dis_fn = converter._get_dis_func(subclass_union) + + base_struct_hook = converter.gen_structure_attrs_fromdict(cl) + + def structure_a(val: dict, _, c=converter, cl=cl) -> cl: + dis_cl = dis_fn(val) + if dis_cl is cl: + return base_struct_hook(val, cl) + return c.structure(val, dis_cl) + + converter.register_structure_hook_func(lambda cls: cls is cl, structure_a) \ No newline at end of file From c431675584a037d563bd439eddc8712b62f60d67 Mon Sep 17 00:00:00 2001 From: Matthieu Melot Date: Thu, 3 Nov 2022 07:36:37 -0400 Subject: [PATCH 03/17] Changes as-is not finished yet --- src/cattrs/strategies/_subclasses.py | 23 +- tests/strategies/test_include_subclasses.py | 242 ++++++++++++++++++++ 2 files changed, 261 insertions(+), 4 deletions(-) create mode 100644 tests/strategies/test_include_subclasses.py diff --git a/src/cattrs/strategies/_subclasses.py b/src/cattrs/strategies/_subclasses.py index 06837502..f18141bc 100644 --- a/src/cattrs/strategies/_subclasses.py +++ b/src/cattrs/strategies/_subclasses.py @@ -4,6 +4,17 @@ from ..converters import Converter + +def _make_subclasses_tree(cl: Type) -> List[Type]: + return [cl] + [ + sscl for scl in cl.__subclasses__() for sscl in _make_subclasses_tree(scl) + ] + + +def _has_subclasses(cl): + return bool(cl.__subclasses__()) + + def include_subclasses( cl: Type, converter: Converter, subclasses: Optional[Tuple[Type]] = None ) -> None: @@ -36,12 +47,16 @@ def unstructure_a(val: cl, c=converter) -> Dict: converter.register_unstructure_hook_func(lambda cls: cls is cl, unstructure_a) if subclasses is not None: - subclass_union = Union[(cl, subclasses)] + parent_subclass_tree = (cl, subclasses) else: - subclass_union = Union[(cl, *cl.__subclasses__())] + parent_subclass_tree = tuple(_make_subclasses_tree(cl)) - dis_fn = converter._get_dis_func(subclass_union) + # for cl in parent_subclass_tree: + # if not _has_subclasses(cl): + # continue + subclass_union = Union[parent_subclass_tree] + dis_fn = converter._get_dis_func(subclass_union) base_struct_hook = converter.gen_structure_attrs_fromdict(cl) def structure_a(val: dict, _, c=converter, cl=cl) -> cl: @@ -50,4 +65,4 @@ def structure_a(val: dict, _, c=converter, cl=cl) -> cl: return base_struct_hook(val, cl) return c.structure(val, dis_cl) - converter.register_structure_hook_func(lambda cls: cls is cl, structure_a) \ No newline at end of file + converter.register_structure_hook_func(lambda cls: cls is cl, structure_a) diff --git a/tests/strategies/test_include_subclasses.py b/tests/strategies/test_include_subclasses.py new file mode 100644 index 00000000..cb81e27d --- /dev/null +++ b/tests/strategies/test_include_subclasses.py @@ -0,0 +1,242 @@ +import collections +import typing +import inspect + +import attr +import pytest + +from cattrs import BaseConverter, Converter +from cattrs.errors import ClassValidationError +from cattrs.strategies._subclasses import _make_subclasses_tree, include_subclasses + + +@attr.define +class Parent: + p: int + + +@attr.define +class Child1(Parent): + c1: int + + +@attr.define +class GrandChild(Child1): + g: int + + +@attr.define +class Child2(Parent): + c2: int + + +@attr.define +class UnionCompose: + a: typing.Union[Parent, Child1, Child2, GrandChild] + + +@attr.define +class NonUnionCompose: + a: Parent + + +@attr.define +class UnionContainer: + a: typing.List[typing.Union[Parent, Child1, Child2, GrandChild]] + + +@attr.define +class NonUnionContainer: + a: typing.List[Parent] + + +@attr.define +class CircularA: + a: int + other: "typing.List[CircularA]" + + +@attr.define +class CircularB(CircularA): + b: int + + +IDS_TO_STRUCT_UNSTRUCT = { + "parent-only": (Parent(1), dict(p=1)), + "child1-only": (Child1(1, 2), dict(p=1, c1=2)), + "grandchild-only": (GrandChild(1, 2, 3), dict(p=1, c1=2, g=3)), + "union-compose-parent": (UnionCompose(Parent(1)), dict(a=dict(p=1))), + "union-compose-child": (UnionCompose(Child1(1, 2)), dict(a=dict(p=1, c1=2))), + "union-compose-grandchild": ( + UnionCompose(GrandChild(1, 2, 3)), + dict(a=(dict(p=1, c1=2, g=3))), + ), + "non-union-compose-parent": (NonUnionCompose(Parent(1)), dict(a=dict(p=1))), + "non-union-compose-child": (NonUnionCompose(Child1(1, 2)), dict(a=dict(p=1, c1=2))), + "non-union-compose-grandchild": ( + NonUnionCompose(GrandChild(1, 2, 3)), + dict(a=(dict(p=1, c1=2, g=3))), + ), + "union-container": ( + UnionContainer([Parent(1), GrandChild(1, 2, 3)]), + dict(a=[dict(p=1), dict(p=1, c1=2, g=3)]), + ), + "non-union-container": ( + NonUnionContainer([Parent(1), GrandChild(1, 2, 3)]), + dict(a=[dict(p=1), dict(p=1, c1=2, g=3)]), + ), +} + + +@pytest.fixture(params=(True, False)) +def conv_w_subclasses(request): + c = Converter() + if request.param: + include_subclasses(Parent, c) + + return c, request.param + + +@pytest.mark.parametrize( + "struct_unstruct", IDS_TO_STRUCT_UNSTRUCT.values(), ids=IDS_TO_STRUCT_UNSTRUCT +) +def test_structuring_with_inheritance( + conv_w_subclasses: typing.Tuple[Converter, bool], struct_unstruct +): + structured, unstructured = struct_unstruct + + converter, included_subclasses = conv_w_subclasses + + if not included_subclasses and isinstance( + structured, (NonUnionContainer, NonUnionCompose) + ): + pytest.xfail( + "Cannot structure subclasses if include_subclasses strategy is not used" + ) + assert converter.structure(unstructured, structured.__class__) == structured + + if structured.__class__ in {Child1, Child2, GrandChild}: + if not included_subclasses: + pytest.xfail( + "Cannot structure subclasses if include_subclasses strategy is not used" + ) + assert converter.structure(unstructured, Parent) == structured + + if structured.__class__ == GrandChild: + assert converter.structure(unstructured, Child1) == structured + + if structured.__class__ in {Parent, Child1, Child2}: + with pytest.raises(ClassValidationError): + converter.structure(unstructured, GrandChild) + + +def test_structure_non_attr_subclass(): + @attr.define + class A: + a: int + + class B(A): + def __init__(self, a: int, b: int): + super().__init__(self, a) + self.b = b + + converter = Converter(include_subclasses=True) + d = dict(a=1, b=2) + with pytest.raises(ValueError, match="has no usable unique attributes"): + converter.structure(d, A) + + +def test_structure_as_union(): + converter = Converter(include_subclasses=True) + the_list = [dict(p=1, c1=2)] + res = converter.structure(the_list, typing.List[typing.Union[Parent, Child1]]) + _show_source(converter, Parent) + _show_source(converter, Child1) + assert res == [Child1(1, 2)] + + +def test_circular_reference(): + c = Converter(include_subclasses=True) + struct = CircularB(a=1, other=[CircularB(a=2, other=[], b=3)], b=4) + unstruct = dict(a=1, other=[dict(a=2, other=[], b=3)], b=4) + + res = c.unstructure(struct) + assert res == unstruct + + res = c.unstructure(struct, CircularA) + assert res == unstruct + + res = c.structure(unstruct, CircularA) + assert res == struct + + +@pytest.mark.parametrize( + "struct_unstruct", IDS_TO_STRUCT_UNSTRUCT.values(), ids=IDS_TO_STRUCT_UNSTRUCT +) +def test_unstructuring_with_inheritance( + conv_w_subclasses: typing.Tuple[Converter, bool], struct_unstruct +): + structured, unstructured = struct_unstruct + converter, included_subclasses = conv_w_subclasses + + if not included_subclasses: + if isinstance(structured, (NonUnionContainer, NonUnionCompose)): + pytest.xfail("Cannot succeed if include_subclasses strategy is not used") + + assert converter.unstructure(structured) == unstructured + + if structured.__class__ in {Child1, Child2, GrandChild}: + if not included_subclasses: + pytest.xfail("Cannot succeed if include_subclasses strategy is not used") + assert converter.unstructure(structured, unstructure_as=Parent) == unstructured + + +def test_unstructuring_unknown_subclass(): + @attr.define + class A: + a: int + + @attr.define + class A1(A): + a1: int + + converter = Converter(include_subclasses=True) + assert converter.unstructure(A1(1, 2), unstructure_as=A) == {"a": 1, "a1": 2} + + @attr.define + class A2(A1): + a2: int + + _show_source(converter, A, "unstructure") + + with pytest.raises(UnknownSubclassError, match="Subclass.*A2.*of.*A1.* is unknown"): + converter.unstructure(A2(1, 2, 3), unstructure_as=A1) + + with pytest.raises(UnknownSubclassError, match="Subclass.*A2.*of.*A.* is unknown"): + converter.unstructure(A2(1, 2, 3), unstructure_as=A) + + +def test_class_tree_generator(): + class P: + pass + + class C1(P): + pass + + class C2(P): + pass + + class GC1(C2): + pass + + class GC2(C2): + pass + + tree_c1 = _make_subclasses_tree(C1) + assert tree_c1 == [C1] + + tree_c2 = _make_subclasses_tree(C2) + assert tree_c2 == [C2, GC1, GC2] + + tree_p = _make_subclasses_tree(P) + assert tree_p == [P, C1, C2, GC1, GC2] From 916381c8d3d593fe8c70012fa05f56e216ac2106 Mon Sep 17 00:00:00 2001 From: Matthieu Melot Date: Mon, 7 Nov 2022 00:23:14 -0500 Subject: [PATCH 04/17] Mind the scope! A closure factory should return only one inner function. If not, there is a risk of messing up all free variables like cl for instance here. All inner generated functions had the last iterated cl value which obviously did not work well... --- src/cattrs/strategies/_subclasses.py | 41 ++++++++++++++++++---------- 1 file changed, 27 insertions(+), 14 deletions(-) diff --git a/src/cattrs/strategies/_subclasses.py b/src/cattrs/strategies/_subclasses.py index f18141bc..50eff163 100644 --- a/src/cattrs/strategies/_subclasses.py +++ b/src/cattrs/strategies/_subclasses.py @@ -1,6 +1,6 @@ """Strategies for customizing subclass behaviors.""" from gc import collect -from typing import Dict, Optional, Tuple, Type, Union, List +from typing import Dict, Optional, Tuple, Type, Union, List, Callable from ..converters import Converter @@ -28,6 +28,27 @@ def include_subclasses( """ # Due to https://github.com/python-attrs/attrs/issues/1047 collect() + + can_handle, unstructure_a = gen_unstructure_handling_pair(converter, cl) + + # This needs to use function dispatch, using singledispatch will again + # match A and all subclasses, which is not what we want. + converter.register_unstructure_hook_func(can_handle, unstructure_a) + + if subclasses is not None: + parent_subclass_tree = (cl, subclasses) + else: + parent_subclass_tree = tuple(_make_subclasses_tree(cl)) + + for cl in parent_subclass_tree: + if not _has_subclasses(cl): + continue + + can_handle, structure_a = gen_structure_handling_pair(converter, cl) + converter.register_structure_hook_func(can_handle, structure_a) + + +def gen_unstructure_handling_pair(converter: Converter, cl: Type): # This hook is for instances of A, but not instances of subclasses. base_hook = converter.gen_unstructure_attrs_fromdict(cl) @@ -42,20 +63,12 @@ def unstructure_a(val: cl, c=converter) -> Dict: return base_hook(val) return c.unstructure(val, unstructure_as=val.__class__) - # This needs to use function dispatch, using singledispatch will again - # match A and all subclasses, which is not what we want. - converter.register_unstructure_hook_func(lambda cls: cls is cl, unstructure_a) - - if subclasses is not None: - parent_subclass_tree = (cl, subclasses) - else: - parent_subclass_tree = tuple(_make_subclasses_tree(cl)) + return (lambda cls: cls is cl, unstructure_a) - # for cl in parent_subclass_tree: - # if not _has_subclasses(cl): - # continue - subclass_union = Union[parent_subclass_tree] +def gen_structure_handling_pair(converter: Converter, cl: Type) -> Tuple[Callable]: + class_tree = _make_subclasses_tree(cl) + subclass_union = Union[tuple(class_tree)] dis_fn = converter._get_dis_func(subclass_union) base_struct_hook = converter.gen_structure_attrs_fromdict(cl) @@ -65,4 +78,4 @@ def structure_a(val: dict, _, c=converter, cl=cl) -> cl: return base_struct_hook(val, cl) return c.structure(val, dis_cl) - converter.register_structure_hook_func(lambda cls: cls is cl, structure_a) + return (lambda cls: cls is cl, structure_a) From f5923277e03120bddb17e39dca1ad64de46f9525 Mon Sep 17 00:00:00 2001 From: Matthieu Melot Date: Mon, 7 Nov 2022 01:13:34 -0500 Subject: [PATCH 05/17] Update unit tests for include_subclasses --- src/cattrs/strategies/_subclasses.py | 23 ++++++---- tests/strategies/test_include_subclasses.py | 51 +++++++++------------ 2 files changed, 34 insertions(+), 40 deletions(-) diff --git a/src/cattrs/strategies/_subclasses.py b/src/cattrs/strategies/_subclasses.py index 50eff163..73f5738d 100644 --- a/src/cattrs/strategies/_subclasses.py +++ b/src/cattrs/strategies/_subclasses.py @@ -29,12 +29,6 @@ def include_subclasses( # Due to https://github.com/python-attrs/attrs/issues/1047 collect() - can_handle, unstructure_a = gen_unstructure_handling_pair(converter, cl) - - # This needs to use function dispatch, using singledispatch will again - # match A and all subclasses, which is not what we want. - converter.register_unstructure_hook_func(can_handle, unstructure_a) - if subclasses is not None: parent_subclass_tree = (cl, subclasses) else: @@ -44,8 +38,17 @@ def include_subclasses( if not _has_subclasses(cl): continue - can_handle, structure_a = gen_structure_handling_pair(converter, cl) - converter.register_structure_hook_func(can_handle, structure_a) + # Unstructuring ... + can_handle_unstruct, unstructure_a = gen_unstructure_handling_pair( + converter, cl + ) + # This needs to use function dispatch, using singledispatch will again + # match A and all subclasses, which is not what we want. + converter.register_unstructure_hook_func(can_handle_unstruct, unstructure_a) + + # Structuring... + can_handle_struct, structure_a = gen_structure_handling_pair(converter, cl) + converter.register_structure_hook_func(can_handle_struct, structure_a) def gen_unstructure_handling_pair(converter: Converter, cl: Type): @@ -67,8 +70,8 @@ def unstructure_a(val: cl, c=converter) -> Dict: def gen_structure_handling_pair(converter: Converter, cl: Type) -> Tuple[Callable]: - class_tree = _make_subclasses_tree(cl) - subclass_union = Union[tuple(class_tree)] + class_tree = tuple(_make_subclasses_tree(cl)) + subclass_union = Union[class_tree] dis_fn = converter._get_dis_func(subclass_union) base_struct_hook = converter.gen_structure_attrs_fromdict(cl) diff --git a/tests/strategies/test_include_subclasses.py b/tests/strategies/test_include_subclasses.py index cb81e27d..0f549041 100644 --- a/tests/strategies/test_include_subclasses.py +++ b/tests/strategies/test_include_subclasses.py @@ -5,7 +5,7 @@ import attr import pytest -from cattrs import BaseConverter, Converter +from cattrs import Converter from cattrs.errors import ClassValidationError from cattrs.strategies._subclasses import _make_subclasses_tree, include_subclasses @@ -88,7 +88,7 @@ class CircularB(CircularA): } -@pytest.fixture(params=(True, False)) +@pytest.fixture(params=(True, False), ids=["with-subclasses", "wo-subclasses"]) def conv_w_subclasses(request): c = Converter() if request.param: @@ -130,33 +130,17 @@ def test_structuring_with_inheritance( converter.structure(unstructured, GrandChild) -def test_structure_non_attr_subclass(): - @attr.define - class A: - a: int - - class B(A): - def __init__(self, a: int, b: int): - super().__init__(self, a) - self.b = b - - converter = Converter(include_subclasses=True) - d = dict(a=1, b=2) - with pytest.raises(ValueError, match="has no usable unique attributes"): - converter.structure(d, A) - - def test_structure_as_union(): - converter = Converter(include_subclasses=True) + converter = Converter() + include_subclasses(Parent, converter) the_list = [dict(p=1, c1=2)] res = converter.structure(the_list, typing.List[typing.Union[Parent, Child1]]) - _show_source(converter, Parent) - _show_source(converter, Child1) assert res == [Child1(1, 2)] def test_circular_reference(): - c = Converter(include_subclasses=True) + c = Converter() + include_subclasses(CircularA, c) struct = CircularB(a=1, other=[CircularB(a=2, other=[], b=3)], b=4) unstruct = dict(a=1, other=[dict(a=2, other=[], b=3)], b=4) @@ -190,8 +174,11 @@ def test_unstructuring_with_inheritance( pytest.xfail("Cannot succeed if include_subclasses strategy is not used") assert converter.unstructure(structured, unstructure_as=Parent) == unstructured + if structured.__class__ == GrandChild: + assert converter.unstructure(structured, unstructure_as=Child1) == unstructured + -def test_unstructuring_unknown_subclass(): +def test_structuring_unstructuring_unknown_subclass(): @attr.define class A: a: int @@ -200,20 +187,24 @@ class A: class A1(A): a1: int - converter = Converter(include_subclasses=True) - assert converter.unstructure(A1(1, 2), unstructure_as=A) == {"a": 1, "a1": 2} + converter = Converter() + include_subclasses(A, converter) + # We define A2 after having created the custom un/structuring functions for A and A1 @attr.define class A2(A1): a2: int - _show_source(converter, A, "unstructure") + # Even if A2 did not exist, unstructuring_as A works: + assert converter.unstructure(A2(1, 2, 3), unstructure_as=A) == dict(a=1, a1=2, a2=3) - with pytest.raises(UnknownSubclassError, match="Subclass.*A2.*of.*A1.* is unknown"): - converter.unstructure(A2(1, 2, 3), unstructure_as=A1) + # This is an known edge case. The result here is not the correct one! It should be + # the same as the previous assert. We leave as-is for now and we document that + # it is preferable to know all subclasses tree before calling include_subclasses + assert converter.unstructure(A2(1, 2, 3), unstructure_as=A1) == {"a": 1, "a1": 2} - with pytest.raises(UnknownSubclassError, match="Subclass.*A2.*of.*A.* is unknown"): - converter.unstructure(A2(1, 2, 3), unstructure_as=A) + # This is another edge-case: the result should be A2(1, 2, 3)... + assert converter.structure(dict(a=1, a1=2, a2=3), A) == A1(1, 2) def test_class_tree_generator(): From 44f3a304fa83804ffb7feb694994ccc53626e19a Mon Sep 17 00:00:00 2001 From: Matthieu Melot Date: Sun, 13 Nov 2022 01:14:59 -0500 Subject: [PATCH 06/17] Handle the subclasses argument for subclass strategy --- src/cattrs/strategies/_subclasses.py | 31 +++++++++++++-------- tests/strategies/test_include_subclasses.py | 20 ++++++++++++- 2 files changed, 38 insertions(+), 13 deletions(-) diff --git a/src/cattrs/strategies/_subclasses.py b/src/cattrs/strategies/_subclasses.py index 73f5738d..4023359e 100644 --- a/src/cattrs/strategies/_subclasses.py +++ b/src/cattrs/strategies/_subclasses.py @@ -11,31 +11,33 @@ def _make_subclasses_tree(cl: Type) -> List[Type]: ] -def _has_subclasses(cl): - return bool(cl.__subclasses__()) +def _has_subclasses(cl: Type, given_subclasses: Tuple[Type]): + actual = set(cl.__subclasses__()) + given = set(given_subclasses) + return bool(actual & given) def include_subclasses( cl: Type, converter: Converter, subclasses: Optional[Tuple[Type]] = None ) -> None: """ - Modify the given converter so that the attrs/dataclass `cl` is - un/structured as if it was a union of itself and all its subclasses - that are defined at the time when this strategy is applied. + Modify the given converter so that the attrs/dataclass `cl` is un/structured as if + it was a union of itself and all its subclasses that are defined at the time when + this strategy is applied. - Subclasses are detected using the `__subclasses__` method, or - they can be explicitly provided. + Subclasses are detected using the `__subclasses__` method, or they can be explicitly + provided. """ # Due to https://github.com/python-attrs/attrs/issues/1047 collect() if subclasses is not None: - parent_subclass_tree = (cl, subclasses) + parent_subclass_tree = (cl,) + subclasses else: parent_subclass_tree = tuple(_make_subclasses_tree(cl)) for cl in parent_subclass_tree: - if not _has_subclasses(cl): + if not _has_subclasses(cl, parent_subclass_tree): continue # Unstructuring ... @@ -47,7 +49,9 @@ def include_subclasses( converter.register_unstructure_hook_func(can_handle_unstruct, unstructure_a) # Structuring... - can_handle_struct, structure_a = gen_structure_handling_pair(converter, cl) + can_handle_struct, structure_a = gen_structure_handling_pair( + converter, cl, parent_subclass_tree + ) converter.register_structure_hook_func(can_handle_struct, structure_a) @@ -69,8 +73,11 @@ def unstructure_a(val: cl, c=converter) -> Dict: return (lambda cls: cls is cl, unstructure_a) -def gen_structure_handling_pair(converter: Converter, cl: Type) -> Tuple[Callable]: - class_tree = tuple(_make_subclasses_tree(cl)) +def gen_structure_handling_pair( + converter: Converter, cl: Type, given_subclasses_tree: Tuple[Type] +) -> Tuple[Callable]: + actual_subclass_tree = tuple(_make_subclasses_tree(cl)) + class_tree = tuple(set(actual_subclass_tree) & set(given_subclasses_tree)) subclass_union = Union[class_tree] dis_fn = converter._get_dis_func(subclass_union) base_struct_hook = converter.gen_structure_attrs_fromdict(cl) diff --git a/tests/strategies/test_include_subclasses.py b/tests/strategies/test_include_subclasses.py index 0f549041..28802d4d 100644 --- a/tests/strategies/test_include_subclasses.py +++ b/tests/strategies/test_include_subclasses.py @@ -5,7 +5,8 @@ import attr import pytest -from cattrs import Converter +from cattrs import Converter, override +from cattrs.gen import make_dict_structure_fn, make_dict_unstructure_fn from cattrs.errors import ClassValidationError from cattrs.strategies._subclasses import _make_subclasses_tree, include_subclasses @@ -207,6 +208,23 @@ class A2(A1): assert converter.structure(dict(a=1, a1=2, a2=3), A) == A1(1, 2) +def test_structuring_with_subclasses_argument(): + c = Converter() + include_subclasses(Parent, c, subclasses=(Child1,)) + + structured_child, unstructured_child = IDS_TO_STRUCT_UNSTRUCT[ + "non-union-compose-child" + ] + assert c.structure(unstructured_child, NonUnionCompose) == structured_child + assert c.unstructure(structured_child) == unstructured_child + + structured_gchild, unstructured_gchild = IDS_TO_STRUCT_UNSTRUCT[ + "non-union-compose-grandchild" + ] + assert c.structure(unstructured_gchild, NonUnionCompose) == structured_child + assert c.unstructure(structured_gchild) == unstructured_gchild + + def test_class_tree_generator(): class P: pass From b0ad5bb1c1677872e279bccab98c505807e3753e Mon Sep 17 00:00:00 2001 From: Matthieu Melot Date: Sun, 13 Nov 2022 01:16:08 -0500 Subject: [PATCH 07/17] Add failing override test with subclasses strategy --- tests/strategies/test_include_subclasses.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/strategies/test_include_subclasses.py b/tests/strategies/test_include_subclasses.py index 28802d4d..1c615eaf 100644 --- a/tests/strategies/test_include_subclasses.py +++ b/tests/strategies/test_include_subclasses.py @@ -225,6 +225,21 @@ def test_structuring_with_subclasses_argument(): assert c.unstructure(structured_gchild) == unstructured_gchild +def test_overrides(): + c = Converter() + overrides = {"p": override(rename="u")} + c.register_unstructure_hook( + Parent, make_dict_unstructure_fn(Parent, c, **overrides) + ) + c.register_structure_hook(Parent, make_dict_structure_fn(Parent, c, **overrides)) + include_subclasses(Parent, c) + + assert c.unstructure(Parent(1)) == {"u": 1} + assert c.structure({"u": 1}, Parent) == Parent(1) + + assert c.unstructure(Child1(1, 2)) == {"u": 1, "c1": 2} + + def test_class_tree_generator(): class P: pass From 36f512defd20929785d8ce44f84814d397dd8862 Mon Sep 17 00:00:00 2001 From: Matthieu Melot Date: Sun, 13 Nov 2022 02:10:09 -0500 Subject: [PATCH 08/17] Added overrides argument in include_subclasses Allows to propagate overrides easily to child classes. --- src/cattrs/strategies/_subclasses.py | 31 ++++++++++++++++----- tests/strategies/test_include_subclasses.py | 7 +---- 2 files changed, 25 insertions(+), 13 deletions(-) diff --git a/src/cattrs/strategies/_subclasses.py b/src/cattrs/strategies/_subclasses.py index 4023359e..a36d6dcd 100644 --- a/src/cattrs/strategies/_subclasses.py +++ b/src/cattrs/strategies/_subclasses.py @@ -3,6 +3,7 @@ from typing import Dict, Optional, Tuple, Type, Union, List, Callable from ..converters import Converter +from ..gen import AttributeOverride, make_dict_structure_fn, make_dict_unstructure_fn def _make_subclasses_tree(cl: Type) -> List[Type]: @@ -18,7 +19,10 @@ def _has_subclasses(cl: Type, given_subclasses: Tuple[Type]): def include_subclasses( - cl: Type, converter: Converter, subclasses: Optional[Tuple[Type]] = None + cl: Type, + converter: Converter, + subclasses: Optional[Tuple[Type]] = None, + overrides: Optional[Dict[str, AttributeOverride]] = None, ) -> None: """ Modify the given converter so that the attrs/dataclass `cl` is un/structured as if @@ -27,6 +31,9 @@ def include_subclasses( Subclasses are detected using the `__subclasses__` method, or they can be explicitly provided. + + overrides is a mapping of some or all the parent class field names to attribute + overrides instantiated with :func:`cattrs.gen.override` """ # Due to https://github.com/python-attrs/attrs/issues/1047 collect() @@ -36,13 +43,16 @@ def include_subclasses( else: parent_subclass_tree = tuple(_make_subclasses_tree(cl)) + if overrides is None: + overrides = {} + for cl in parent_subclass_tree: if not _has_subclasses(cl, parent_subclass_tree): continue # Unstructuring ... can_handle_unstruct, unstructure_a = gen_unstructure_handling_pair( - converter, cl + converter, cl, overrides ) # This needs to use function dispatch, using singledispatch will again # match A and all subclasses, which is not what we want. @@ -50,14 +60,18 @@ def include_subclasses( # Structuring... can_handle_struct, structure_a = gen_structure_handling_pair( - converter, cl, parent_subclass_tree + converter, cl, parent_subclass_tree, overrides ) converter.register_structure_hook_func(can_handle_struct, structure_a) -def gen_unstructure_handling_pair(converter: Converter, cl: Type): +def gen_unstructure_handling_pair( + converter: Converter, + cl: Type, + overrides: Optional[Dict[str, AttributeOverride]] = None, +): # This hook is for instances of A, but not instances of subclasses. - base_hook = converter.gen_unstructure_attrs_fromdict(cl) + base_hook = make_dict_unstructure_fn(cl, converter, **overrides) def unstructure_a(val: cl, c=converter) -> Dict: """ @@ -74,13 +88,16 @@ def unstructure_a(val: cl, c=converter) -> Dict: def gen_structure_handling_pair( - converter: Converter, cl: Type, given_subclasses_tree: Tuple[Type] + converter: Converter, + cl: Type, + given_subclasses_tree: Tuple[Type], + overrides: Optional[Dict[str, AttributeOverride]] = None, ) -> Tuple[Callable]: actual_subclass_tree = tuple(_make_subclasses_tree(cl)) class_tree = tuple(set(actual_subclass_tree) & set(given_subclasses_tree)) subclass_union = Union[class_tree] dis_fn = converter._get_dis_func(subclass_union) - base_struct_hook = converter.gen_structure_attrs_fromdict(cl) + base_struct_hook = make_dict_structure_fn(cl, converter, **overrides) def structure_a(val: dict, _, c=converter, cl=cl) -> cl: dis_cl = dis_fn(val) diff --git a/tests/strategies/test_include_subclasses.py b/tests/strategies/test_include_subclasses.py index 1c615eaf..8433c983 100644 --- a/tests/strategies/test_include_subclasses.py +++ b/tests/strategies/test_include_subclasses.py @@ -227,12 +227,7 @@ def test_structuring_with_subclasses_argument(): def test_overrides(): c = Converter() - overrides = {"p": override(rename="u")} - c.register_unstructure_hook( - Parent, make_dict_unstructure_fn(Parent, c, **overrides) - ) - c.register_structure_hook(Parent, make_dict_structure_fn(Parent, c, **overrides)) - include_subclasses(Parent, c) + include_subclasses(Parent, c, overrides={"p": override(rename="u")}) assert c.unstructure(Parent(1)) == {"u": 1} assert c.structure({"u": 1}, Parent) == Parent(1) From 2f8b66c4ea703ba7a83e2c413f00510544005557 Mon Sep 17 00:00:00 2001 From: Matthieu Melot Date: Thu, 5 Jan 2023 09:23:08 -0500 Subject: [PATCH 09/17] Initial work of inlcue_subclasses with union strategy * Adapted the code provided by Tin * New test parameters Some unstructuring tests with the union strategy fail: infinite recursion error. --- src/cattrs/strategies/_subclasses.py | 81 ++++++++++------- tests/strategies/test_include_subclasses.py | 96 ++++++++++++++++----- 2 files changed, 125 insertions(+), 52 deletions(-) diff --git a/src/cattrs/strategies/_subclasses.py b/src/cattrs/strategies/_subclasses.py index a36d6dcd..e4ad915f 100644 --- a/src/cattrs/strategies/_subclasses.py +++ b/src/cattrs/strategies/_subclasses.py @@ -1,8 +1,8 @@ """Strategies for customizing subclass behaviors.""" from gc import collect -from typing import Dict, Optional, Tuple, Type, Union, List, Callable +from typing import Dict, Optional, Tuple, Type, Union, List, Callable, Any -from ..converters import Converter +from ..converters import Converter, BaseConverter from ..gen import AttributeOverride, make_dict_structure_fn, make_dict_unstructure_fn @@ -18,10 +18,18 @@ def _has_subclasses(cl: Type, given_subclasses: Tuple[Type]): return bool(actual & given) +def _get_union_type(cl: Type, given_subclasses_tree: Tuple[Type]) -> Type: + actual_subclass_tree = tuple(_make_subclasses_tree(cl)) + class_tree = tuple(set(actual_subclass_tree) & set(given_subclasses_tree)) + union_type = Union[class_tree] + return union_type + + def include_subclasses( cl: Type, converter: Converter, subclasses: Optional[Tuple[Type]] = None, + union_strategy: Optional[Callable[[Any, BaseConverter], Any]] = None, overrides: Optional[Dict[str, AttributeOverride]] = None, ) -> None: """ @@ -37,7 +45,6 @@ def include_subclasses( """ # Due to https://github.com/python-attrs/attrs/issues/1047 collect() - if subclasses is not None: parent_subclass_tree = (cl,) + subclasses else: @@ -46,63 +53,79 @@ def include_subclasses( if overrides is None: overrides = {} + # The iteration approach is required if subclasses are more than one level deep: ? n; for cl in parent_subclass_tree: if not _has_subclasses(cl, parent_subclass_tree): continue - # Unstructuring ... - can_handle_unstruct, unstructure_a = gen_unstructure_handling_pair( - converter, cl, overrides - ) + # We re-create a reduced union type to handle the following case: + # >>> converter.structure(d, as=Child) + # and the `as=Child` will be transformed to a union type of itself and its + # subtypes, that way we guarantee that the returned object will not be the + # parent. + subclass_union = _get_union_type(cl, parent_subclass_tree) + + if union_strategy is None: + unstruct_hook = gen_unstructure_hook(converter, cl, overrides) + struct_hook = gen_structure_hook(converter, cl, subclass_union, overrides) + else: + union_strategy(subclass_union, converter) + unstruct_hook = converter._unstructure_func.dispatch(subclass_union) + struct_hook = converter._union_struct_registry[subclass_union] + + # Note: the closure approach is needed due to python scoping rule. If we define + # the lambda here, the last class in the iteration will be used in all lambdas. + cls_is_cl = gen_cls_is_cl(cl) + # This needs to use function dispatch, using singledispatch will again # match A and all subclasses, which is not what we want. - converter.register_unstructure_hook_func(can_handle_unstruct, unstructure_a) - - # Structuring... - can_handle_struct, structure_a = gen_structure_handling_pair( - converter, cl, parent_subclass_tree, overrides - ) - converter.register_structure_hook_func(can_handle_struct, structure_a) + converter.register_unstructure_hook_func(cls_is_cl, unstruct_hook) + converter.register_structure_hook_func(cls_is_cl, struct_hook) -def gen_unstructure_handling_pair( +def gen_unstructure_hook( converter: Converter, cl: Type, overrides: Optional[Dict[str, AttributeOverride]] = None, ): - # This hook is for instances of A, but not instances of subclasses. + # This hook is for instances of the class cl, but not instances of subclasses. base_hook = make_dict_unstructure_fn(cl, converter, **overrides) - def unstructure_a(val: cl, c=converter) -> Dict: + def unstructure_hook(val: cl, c=converter) -> Dict: """ - If val is an instance of `A`, use the hook. + If val is an instance of the class `cl`, use the hook. - If val is an instance of a subclass, dispatch on its exact - runtime type. + If val is an instance of a subclass, dispatch on its exact runtime type. """ if val.__class__ is cl: return base_hook(val) return c.unstructure(val, unstructure_as=val.__class__) - return (lambda cls: cls is cl, unstructure_a) + return unstructure_hook -def gen_structure_handling_pair( +def gen_structure_hook( converter: Converter, cl: Type, - given_subclasses_tree: Tuple[Type], + subclass_union: Type, overrides: Optional[Dict[str, AttributeOverride]] = None, -) -> Tuple[Callable]: - actual_subclass_tree = tuple(_make_subclasses_tree(cl)) - class_tree = tuple(set(actual_subclass_tree) & set(given_subclasses_tree)) - subclass_union = Union[class_tree] +) -> Callable: dis_fn = converter._get_dis_func(subclass_union) base_struct_hook = make_dict_structure_fn(cl, converter, **overrides) - def structure_a(val: dict, _, c=converter, cl=cl) -> cl: + def structure_hook(val: dict, _, c=converter, cl=cl) -> cl: + """ + If val is an instance of the class `cl`, use the hook. + + If val is an instance of a subclass, dispatch on its exact runtime type. + """ dis_cl = dis_fn(val) if dis_cl is cl: return base_struct_hook(val, cl) return c.structure(val, dis_cl) - return (lambda cls: cls is cl, structure_a) + return structure_hook + + +def gen_cls_is_cl(cl): + return lambda cls: cls is cl diff --git a/tests/strategies/test_include_subclasses.py b/tests/strategies/test_include_subclasses.py index 8433c983..483ad536 100644 --- a/tests/strategies/test_include_subclasses.py +++ b/tests/strategies/test_include_subclasses.py @@ -1,14 +1,14 @@ -import collections import typing -import inspect +from functools import partial +from copy import deepcopy import attr import pytest from cattrs import Converter, override -from cattrs.gen import make_dict_structure_fn, make_dict_unstructure_fn from cattrs.errors import ClassValidationError -from cattrs.strategies._subclasses import _make_subclasses_tree, include_subclasses +from cattrs.strategies._subclasses import _make_subclasses_tree +from cattrs.strategies import configure_tagged_union, include_subclasses @attr.define @@ -62,38 +62,81 @@ class CircularB(CircularA): b: int +def _remove_type_name(unstructured: typing.Union[typing.Dict, typing.List]): + if isinstance(unstructured, list): + iterator = unstructured + elif isinstance(unstructured, dict): + if "type_name" in unstructured: + unstructured.pop("type_name") + iterator = unstructured.values() + for item in iterator: + if isinstance(item, (list, dict)): + _remove_type_name(item) + return unstructured + + IDS_TO_STRUCT_UNSTRUCT = { - "parent-only": (Parent(1), dict(p=1)), - "child1-only": (Child1(1, 2), dict(p=1, c1=2)), - "grandchild-only": (GrandChild(1, 2, 3), dict(p=1, c1=2, g=3)), - "union-compose-parent": (UnionCompose(Parent(1)), dict(a=dict(p=1))), - "union-compose-child": (UnionCompose(Child1(1, 2)), dict(a=dict(p=1, c1=2))), + "parent-only": (Parent(1), dict(p=1, type_name="Parent")), + "child1-only": (Child1(1, 2), dict(p=1, c1=2, type_name="Child1")), + "grandchild-only": ( + GrandChild(1, 2, 3), + dict(p=1, c1=2, g=3, type_name="GrandChild"), + ), + "union-compose-parent": ( + UnionCompose(Parent(1)), + dict(a=dict(p=1, type_name="Parent")), + ), + "union-compose-child": ( + UnionCompose(Child1(1, 2)), + dict(a=dict(p=1, c1=2, type_name="Child1")), + ), "union-compose-grandchild": ( UnionCompose(GrandChild(1, 2, 3)), - dict(a=(dict(p=1, c1=2, g=3))), + dict(a=(dict(p=1, c1=2, g=3, type_name="GrandChild"))), + ), + "non-union-compose-parent": ( + NonUnionCompose(Parent(1)), + dict(a=dict(p=1, type_name="Parent")), + ), + "non-union-compose-child": ( + NonUnionCompose(Child1(1, 2)), + dict(a=dict(p=1, c1=2, type_name="Child1")), ), - "non-union-compose-parent": (NonUnionCompose(Parent(1)), dict(a=dict(p=1))), - "non-union-compose-child": (NonUnionCompose(Child1(1, 2)), dict(a=dict(p=1, c1=2))), "non-union-compose-grandchild": ( NonUnionCompose(GrandChild(1, 2, 3)), - dict(a=(dict(p=1, c1=2, g=3))), + dict(a=(dict(p=1, c1=2, g=3, type_name="GrandChild"))), ), "union-container": ( UnionContainer([Parent(1), GrandChild(1, 2, 3)]), - dict(a=[dict(p=1), dict(p=1, c1=2, g=3)]), + dict( + a=[ + dict(p=1, type_name="Parent"), + dict(p=1, c1=2, g=3, type_name="GrandChild"), + ] + ), ), "non-union-container": ( NonUnionContainer([Parent(1), GrandChild(1, 2, 3)]), - dict(a=[dict(p=1), dict(p=1, c1=2, g=3)]), + dict( + a=[ + dict(p=1, type_name="Parent"), + dict(p=1, c1=2, g=3, type_name="GrandChild"), + ] + ), ), } -@pytest.fixture(params=(True, False), ids=["with-subclasses", "wo-subclasses"]) +@pytest.fixture( + params=["with-subclasses", "with-subclasses-and-union", "wo-subclasses"] +) def conv_w_subclasses(request): c = Converter() - if request.param: + if request.param == "with-subclasses": include_subclasses(Parent, c) + elif request.param == "with-subclasses-and-union": + union_strategy = partial(configure_tagged_union, tag_name="type_name") + include_subclasses(Parent, c, union_strategy=union_strategy) return c, request.param @@ -106,9 +149,11 @@ def test_structuring_with_inheritance( ): structured, unstructured = struct_unstruct - converter, included_subclasses = conv_w_subclasses + converter, included_subclasses_param = conv_w_subclasses + if included_subclasses_param != "with-subclasses-and-union": + unstructured = _remove_type_name(deepcopy(unstructured)) - if not included_subclasses and isinstance( + if "wo-subclasses" in included_subclasses_param and isinstance( structured, (NonUnionContainer, NonUnionCompose) ): pytest.xfail( @@ -117,7 +162,7 @@ def test_structuring_with_inheritance( assert converter.structure(unstructured, structured.__class__) == structured if structured.__class__ in {Child1, Child2, GrandChild}: - if not included_subclasses: + if "wo-subclasses" in included_subclasses_param: pytest.xfail( "Cannot structure subclasses if include_subclasses strategy is not used" ) @@ -162,16 +207,19 @@ def test_unstructuring_with_inheritance( conv_w_subclasses: typing.Tuple[Converter, bool], struct_unstruct ): structured, unstructured = struct_unstruct - converter, included_subclasses = conv_w_subclasses + converter, included_subclasses_param = conv_w_subclasses - if not included_subclasses: + if "wo-subclasses" in included_subclasses_param: if isinstance(structured, (NonUnionContainer, NonUnionCompose)): pytest.xfail("Cannot succeed if include_subclasses strategy is not used") + if included_subclasses_param != "with-subclasses-and-union": + unstructured = _remove_type_name(deepcopy(unstructured)) + assert converter.unstructure(structured) == unstructured if structured.__class__ in {Child1, Child2, GrandChild}: - if not included_subclasses: + if "wo-subclasses" in included_subclasses_param: pytest.xfail("Cannot succeed if include_subclasses strategy is not used") assert converter.unstructure(structured, unstructure_as=Parent) == unstructured @@ -215,12 +263,14 @@ def test_structuring_with_subclasses_argument(): structured_child, unstructured_child = IDS_TO_STRUCT_UNSTRUCT[ "non-union-compose-child" ] + unstructured_child = _remove_type_name(deepcopy(unstructured_child)) assert c.structure(unstructured_child, NonUnionCompose) == structured_child assert c.unstructure(structured_child) == unstructured_child structured_gchild, unstructured_gchild = IDS_TO_STRUCT_UNSTRUCT[ "non-union-compose-grandchild" ] + unstructured_gchild = _remove_type_name(deepcopy(unstructured_gchild)) assert c.structure(unstructured_gchild, NonUnionCompose) == structured_child assert c.unstructure(structured_gchild) == unstructured_gchild From 04dfc2a634dad403c5b96d0b51ff8eb26ade0765 Mon Sep 17 00:00:00 2001 From: Matthieu Melot Date: Thu, 5 Jan 2023 20:14:25 -0500 Subject: [PATCH 10/17] Structuring / unstructuring work with tagged union strategy Overrides still need to be implemented. --- src/cattrs/strategies/_subclasses.py | 43 ++++++++++++-------- src/cattrs/strategies/_unions.py | 19 +++++++-- tests/strategies/test_include_subclasses.py | 44 +++++++++++++++------ 3 files changed, 74 insertions(+), 32 deletions(-) diff --git a/src/cattrs/strategies/_subclasses.py b/src/cattrs/strategies/_subclasses.py index e4ad915f..cda6043d 100644 --- a/src/cattrs/strategies/_subclasses.py +++ b/src/cattrs/strategies/_subclasses.py @@ -1,6 +1,6 @@ """Strategies for customizing subclass behaviors.""" from gc import collect -from typing import Dict, Optional, Tuple, Type, Union, List, Callable, Any +from typing import Dict, Optional, Tuple, Type, Union, List, Callable, Any, get_args from ..converters import Converter, BaseConverter from ..gen import AttributeOverride, make_dict_structure_fn, make_dict_unstructure_fn @@ -53,33 +53,46 @@ def include_subclasses( if overrides is None: overrides = {} - # The iteration approach is required if subclasses are more than one level deep: ? n; - for cl in parent_subclass_tree: + # The iteration approach is required if subclasses are more than one level deep: + for i, cl in enumerate(parent_subclass_tree): if not _has_subclasses(cl, parent_subclass_tree): continue # We re-create a reduced union type to handle the following case: - # >>> converter.structure(d, as=Child) - # and the `as=Child` will be transformed to a union type of itself and its - # subtypes, that way we guarantee that the returned object will not be the - # parent. + # + # converter.structure(d, as=Child) + # + # In the above, the `as=Child` argument will be transformed to a union type of + # itself and its subtypes, that way we guarantee that the returned object will + # not be the parent. subclass_union = _get_union_type(cl, parent_subclass_tree) + def cls_is_cl(cls, _cl=cl): + return cls is _cl + if union_strategy is None: unstruct_hook = gen_unstructure_hook(converter, cl, overrides) + unstruct_predicate = cls_is_cl struct_hook = gen_structure_hook(converter, cl, subclass_union, overrides) else: union_strategy(subclass_union, converter) - unstruct_hook = converter._unstructure_func.dispatch(subclass_union) - struct_hook = converter._union_struct_registry[subclass_union] + if i == 0: - # Note: the closure approach is needed due to python scoping rule. If we define - # the lambda here, the last class in the iteration will be used in all lambdas. - cls_is_cl = gen_cls_is_cl(cl) + def cls_is_in_union(cls, _union_classes=get_args(subclass_union)): + return cls in _union_classes + + unstruct_hook = converter._unstructure_func.dispatch(subclass_union) + unstruct_predicate = cls_is_in_union + else: + unstruct_hook = None + unstruct_predicate = None + struct_hook = converter._union_struct_registry[subclass_union] # This needs to use function dispatch, using singledispatch will again # match A and all subclasses, which is not what we want. - converter.register_unstructure_hook_func(cls_is_cl, unstruct_hook) + if unstruct_hook is not None: + converter.register_unstructure_hook_func(unstruct_predicate, unstruct_hook) + converter.register_structure_hook_func(cls_is_cl, struct_hook) @@ -125,7 +138,3 @@ def structure_hook(val: dict, _, c=converter, cl=cl) -> cl: return c.structure(val, dis_cl) return structure_hook - - -def gen_cls_is_cl(cl): - return lambda cls: cls is cl diff --git a/src/cattrs/strategies/_unions.py b/src/cattrs/strategies/_unions.py index 422de5ff..1e017381 100644 --- a/src/cattrs/strategies/_unions.py +++ b/src/cattrs/strategies/_unions.py @@ -42,14 +42,21 @@ def configure_tagged_union( """ args = union.__args__ tag_to_hook = {} + exact_cl_unstruct_hooks = {} for cl in args: tag = tag_generator(cl) - handler = converter._structure_func.dispatch(cl) + struct_handler = converter._structure_func.dispatch(cl) + unstruct_handler = converter._unstructure_func.dispatch(cl) - def structure_union_member(val: dict, _cl=cl, _h=handler) -> cl: + def structure_union_member(val: dict, _cl=cl, _h=struct_handler) -> cl: return _h(val, _cl) + def unstructure_union_member(val: union, _h=unstruct_handler) -> dict: + return _h(val) + tag_to_hook[tag] = structure_union_member + exact_cl_unstruct_hooks[cl] = unstructure_union_member + cl_to_tag = {cl: tag_generator(cl) for cl in args} if default is not NOTHING: @@ -62,9 +69,13 @@ def structure_default(val: dict, _cl=default, _h=default_handler): cl_to_tag = defaultdict(lambda: default, cl_to_tag) def unstructure_tagged_union( - val: union, _c=converter, _cl_to_tag=cl_to_tag, _tag_name=tag_name + val: union, + _exact_cl_unstruct_hooks=exact_cl_unstruct_hooks, + _cl_to_tag=cl_to_tag, + _tag_name=tag_name, ) -> Dict: - res = _c.unstructure(val) + + res = _exact_cl_unstruct_hooks[val.__class__](val) res[_tag_name] = _cl_to_tag[val.__class__] return res diff --git a/tests/strategies/test_include_subclasses.py b/tests/strategies/test_include_subclasses.py index 483ad536..e23f3fba 100644 --- a/tests/strategies/test_include_subclasses.py +++ b/tests/strategies/test_include_subclasses.py @@ -78,6 +78,7 @@ def _remove_type_name(unstructured: typing.Union[typing.Dict, typing.List]): IDS_TO_STRUCT_UNSTRUCT = { "parent-only": (Parent(1), dict(p=1, type_name="Parent")), "child1-only": (Child1(1, 2), dict(p=1, c1=2, type_name="Child1")), + "child2-only": (Child2(1, 2), dict(p=1, c2=2, type_name="Child2")), "grandchild-only": ( GrandChild(1, 2, 3), dict(p=1, c1=2, g=3, type_name="GrandChild"), @@ -128,13 +129,13 @@ def _remove_type_name(unstructured: typing.Union[typing.Dict, typing.List]): @pytest.fixture( - params=["with-subclasses", "with-subclasses-and-union", "wo-subclasses"] + params=["with-subclasses", "with-subclasses-and-tagged-union", "wo-subclasses"] ) def conv_w_subclasses(request): c = Converter() if request.param == "with-subclasses": include_subclasses(Parent, c) - elif request.param == "with-subclasses-and-union": + elif request.param == "with-subclasses-and-tagged-union": union_strategy = partial(configure_tagged_union, tag_name="type_name") include_subclasses(Parent, c, union_strategy=union_strategy) @@ -150,7 +151,7 @@ def test_structuring_with_inheritance( structured, unstructured = struct_unstruct converter, included_subclasses_param = conv_w_subclasses - if included_subclasses_param != "with-subclasses-and-union": + if included_subclasses_param != "with-subclasses-and-tagged-union": unstructured = _remove_type_name(deepcopy(unstructured)) if "wo-subclasses" in included_subclasses_param and isinstance( @@ -213,7 +214,7 @@ def test_unstructuring_with_inheritance( if isinstance(structured, (NonUnionContainer, NonUnionCompose)): pytest.xfail("Cannot succeed if include_subclasses strategy is not used") - if included_subclasses_param != "with-subclasses-and-union": + if included_subclasses_param != "with-subclasses-and-tagged-union": unstructured = _remove_type_name(deepcopy(unstructured)) assert converter.unstructure(structured) == unstructured @@ -275,14 +276,35 @@ def test_structuring_with_subclasses_argument(): assert c.unstructure(structured_gchild) == unstructured_gchild -def test_overrides(): +@pytest.mark.parametrize( + "struct_unstruct", ["parent-only", "child1-only", "child2-only", "grandchild-only"] +) +@pytest.mark.parametrize( + "with_union_strategy", + [True, False], + ids=["with-union-strategy", "wo-union-strategy"], +) +def test_overrides(with_union_strategy: bool, struct_unstruct: str): c = Converter() - include_subclasses(Parent, c, overrides={"p": override(rename="u")}) - - assert c.unstructure(Parent(1)) == {"u": 1} - assert c.structure({"u": 1}, Parent) == Parent(1) - - assert c.unstructure(Child1(1, 2)) == {"u": 1, "c1": 2} + union_strategy = ( + partial(configure_tagged_union, tag_name="type_name") + if with_union_strategy + else None + ) + include_subclasses( + Parent, c, overrides={"p": override(rename="u")}, union_strategy=union_strategy + ) + + structured, unstructured = IDS_TO_STRUCT_UNSTRUCT[struct_unstruct] + unstructured = unstructured.copy() + val = unstructured.pop("p") + unstructured["u"] = val + if not with_union_strategy: + unstructured = _remove_type_name(unstructured) + + assert c.unstructure(structured) == unstructured + assert c.structure(unstructured, Parent) == structured + assert c.structure(unstructured, structured.__class__) == structured def test_class_tree_generator(): From f801ed50cca566f08ad4384cbb447d908a8b8dc5 Mon Sep 17 00:00:00 2001 From: Matthieu Melot Date: Fri, 6 Jan 2023 12:00:54 -0500 Subject: [PATCH 11/17] Fixed overrides when there is no union strategy. The overrides with union strategy still fail though. Restructured the code in add_subclasses: * Skipping the classes without subclasses broke the overrides (without union strategy) and also the unstructuring of unknown class. * The hook functions are created inline and the closure variables are passed as default argument to workaround the scoping problems encontered before. --- src/cattrs/strategies/_subclasses.py | 122 +++++++++++--------- tests/strategies/test_include_subclasses.py | 18 +-- 2 files changed, 77 insertions(+), 63 deletions(-) diff --git a/src/cattrs/strategies/_subclasses.py b/src/cattrs/strategies/_subclasses.py index cda6043d..f54e7d84 100644 --- a/src/cattrs/strategies/_subclasses.py +++ b/src/cattrs/strategies/_subclasses.py @@ -18,10 +18,13 @@ def _has_subclasses(cl: Type, given_subclasses: Tuple[Type]): return bool(actual & given) -def _get_union_type(cl: Type, given_subclasses_tree: Tuple[Type]) -> Type: +def _get_union_type(cl: Type, given_subclasses_tree: Tuple[Type]) -> Optional[Type]: actual_subclass_tree = tuple(_make_subclasses_tree(cl)) class_tree = tuple(set(actual_subclass_tree) & set(given_subclasses_tree)) - union_type = Union[class_tree] + if len(class_tree) >= 2: + union_type = Union[class_tree] + else: + union_type = None return union_type @@ -55,9 +58,6 @@ def include_subclasses( # The iteration approach is required if subclasses are more than one level deep: for i, cl in enumerate(parent_subclass_tree): - if not _has_subclasses(cl, parent_subclass_tree): - continue - # We re-create a reduced union type to handle the following case: # # converter.structure(d, as=Child) @@ -70,15 +70,69 @@ def include_subclasses( def cls_is_cl(cls, _cl=cl): return cls is _cl + base_struct_hook = make_dict_structure_fn(cl, converter, **overrides) + base_unstruct_hook = make_dict_unstructure_fn(cl, converter, **overrides) + if union_strategy is None: - unstruct_hook = gen_unstructure_hook(converter, cl, overrides) + if subclass_union is None: + + def struct_hook( + val: dict, _, _cl=cl, _base_hook=base_struct_hook + ) -> cl: + return _base_hook(val, _cl) + + else: + dis_fn = converter._get_dis_func(subclass_union) + + def struct_hook( + val: dict, + _, + _c=converter, + _cl=cl, + _base_hook=base_struct_hook, + _dis_fn=dis_fn, + ) -> cl: + """ + If val is disambiguated to the class `cl`, use its base hook. + + If val is disambiguated to a subclass, dispatch on its exact runtime + type. + """ + dis_cl = _dis_fn(val) + if dis_cl is _cl: + return _base_hook(val, _cl) + return _c.structure(val, dis_cl) + + def unstruct_hook( + val: parent_subclass_tree[0], + _c=converter, + _cl=cl, + _base_hook=base_unstruct_hook, + ) -> Dict: + """ + If val is an instance of the class `cl`, use the hook. + + If val is an instance of a subclass, dispatch on its exact runtime type. + """ + if val.__class__ is _cl: + return _base_hook(val) + return _c.unstructure(val, unstructure_as=val.__class__) + unstruct_predicate = cls_is_cl - struct_hook = gen_structure_hook(converter, cl, subclass_union, overrides) else: - union_strategy(subclass_union, converter) - if i == 0: + if subclass_union is not None: + union_strategy(subclass_union, converter) + struct_hook = converter._union_struct_registry[subclass_union] + else: + struct_hook = None + + if i == 0 and subclass_union is not None: + if subclass_union is not None: + union_classes = get_args(subclass_union) + else: + union_classes = () - def cls_is_in_union(cls, _union_classes=get_args(subclass_union)): + def cls_is_in_union(cls, _union_classes=union_classes): return cls in _union_classes unstruct_hook = converter._unstructure_func.dispatch(subclass_union) @@ -86,55 +140,11 @@ def cls_is_in_union(cls, _union_classes=get_args(subclass_union)): else: unstruct_hook = None unstruct_predicate = None - struct_hook = converter._union_struct_registry[subclass_union] # This needs to use function dispatch, using singledispatch will again # match A and all subclasses, which is not what we want. if unstruct_hook is not None: converter.register_unstructure_hook_func(unstruct_predicate, unstruct_hook) - converter.register_structure_hook_func(cls_is_cl, struct_hook) - - -def gen_unstructure_hook( - converter: Converter, - cl: Type, - overrides: Optional[Dict[str, AttributeOverride]] = None, -): - # This hook is for instances of the class cl, but not instances of subclasses. - base_hook = make_dict_unstructure_fn(cl, converter, **overrides) - - def unstructure_hook(val: cl, c=converter) -> Dict: - """ - If val is an instance of the class `cl`, use the hook. - - If val is an instance of a subclass, dispatch on its exact runtime type. - """ - if val.__class__ is cl: - return base_hook(val) - return c.unstructure(val, unstructure_as=val.__class__) - - return unstructure_hook - - -def gen_structure_hook( - converter: Converter, - cl: Type, - subclass_union: Type, - overrides: Optional[Dict[str, AttributeOverride]] = None, -) -> Callable: - dis_fn = converter._get_dis_func(subclass_union) - base_struct_hook = make_dict_structure_fn(cl, converter, **overrides) - - def structure_hook(val: dict, _, c=converter, cl=cl) -> cl: - """ - If val is an instance of the class `cl`, use the hook. - - If val is an instance of a subclass, dispatch on its exact runtime type. - """ - dis_cl = dis_fn(val) - if dis_cl is cl: - return base_struct_hook(val, cl) - return c.structure(val, dis_cl) - - return structure_hook + if struct_hook is not None: + converter.register_structure_hook_func(cls_is_cl, struct_hook) diff --git a/tests/strategies/test_include_subclasses.py b/tests/strategies/test_include_subclasses.py index e23f3fba..a76c3ea1 100644 --- a/tests/strategies/test_include_subclasses.py +++ b/tests/strategies/test_include_subclasses.py @@ -248,12 +248,16 @@ class A2(A1): # Even if A2 did not exist, unstructuring_as A works: assert converter.unstructure(A2(1, 2, 3), unstructure_as=A) == dict(a=1, a1=2, a2=3) - # This is an known edge case. The result here is not the correct one! It should be - # the same as the previous assert. We leave as-is for now and we document that - # it is preferable to know all subclasses tree before calling include_subclasses - assert converter.unstructure(A2(1, 2, 3), unstructure_as=A1) == {"a": 1, "a1": 2} - - # This is another edge-case: the result should be A2(1, 2, 3)... + # As well as when unstructuring as A1, in other words, unstructuring works for + # unknown classes. + assert converter.unstructure(A2(1, 2, 3), unstructure_as=A1) == { + "a": 1, + "a1": 2, + "a2": 3, + } + + # But as expected, structuring unknown classes as their parent fails to give the + # correct answer. This is a documented drawback, we just confirm it. assert converter.structure(dict(a=1, a1=2, a2=3), A) == A1(1, 2) @@ -300,7 +304,7 @@ def test_overrides(with_union_strategy: bool, struct_unstruct: str): val = unstructured.pop("p") unstructured["u"] = val if not with_union_strategy: - unstructured = _remove_type_name(unstructured) + unstructured.pop("type_name") assert c.unstructure(structured) == unstructured assert c.structure(unstructured, Parent) == structured From 86890190504e94f95e53e543cc419be38001113e Mon Sep 17 00:00:00 2001 From: Matthieu Melot Date: Fri, 6 Jan 2023 13:47:49 -0500 Subject: [PATCH 12/17] Overrides work also with union_strategy --- src/cattrs/strategies/_subclasses.py | 158 ++++++++++++++++----------- 1 file changed, 93 insertions(+), 65 deletions(-) diff --git a/src/cattrs/strategies/_subclasses.py b/src/cattrs/strategies/_subclasses.py index f54e7d84..6bad17b9 100644 --- a/src/cattrs/strategies/_subclasses.py +++ b/src/cattrs/strategies/_subclasses.py @@ -56,8 +56,24 @@ def include_subclasses( if overrides is None: overrides = {} + if union_strategy is None: + _include_subclasses_without_union_strategy( + cl, converter, parent_subclass_tree, overrides + ) + else: + _include_subclasses_with_union_strategy( + converter, parent_subclass_tree, union_strategy, overrides + ) + + +def _include_subclasses_without_union_strategy( + cl, + converter: Converter, + parent_subclass_tree: Tuple[Type], + overrides: Dict[str, AttributeOverride], +): # The iteration approach is required if subclasses are more than one level deep: - for i, cl in enumerate(parent_subclass_tree): + for cl in parent_subclass_tree: # We re-create a reduced union type to handle the following case: # # converter.structure(d, as=Child) @@ -73,78 +89,90 @@ def cls_is_cl(cls, _cl=cl): base_struct_hook = make_dict_structure_fn(cl, converter, **overrides) base_unstruct_hook = make_dict_unstructure_fn(cl, converter, **overrides) - if union_strategy is None: - if subclass_union is None: + if subclass_union is None: - def struct_hook( - val: dict, _, _cl=cl, _base_hook=base_struct_hook - ) -> cl: - return _base_hook(val, _cl) + def struct_hook(val: dict, _, _cl=cl, _base_hook=base_struct_hook) -> cl: + return _base_hook(val, _cl) + + else: + dis_fn = converter._get_dis_func(subclass_union) - else: - dis_fn = converter._get_dis_func(subclass_union) - - def struct_hook( - val: dict, - _, - _c=converter, - _cl=cl, - _base_hook=base_struct_hook, - _dis_fn=dis_fn, - ) -> cl: - """ - If val is disambiguated to the class `cl`, use its base hook. - - If val is disambiguated to a subclass, dispatch on its exact runtime - type. - """ - dis_cl = _dis_fn(val) - if dis_cl is _cl: - return _base_hook(val, _cl) - return _c.structure(val, dis_cl) - - def unstruct_hook( - val: parent_subclass_tree[0], + def struct_hook( + val: dict, + _, _c=converter, _cl=cl, - _base_hook=base_unstruct_hook, - ) -> Dict: + _base_hook=base_struct_hook, + _dis_fn=dis_fn, + ) -> cl: """ - If val is an instance of the class `cl`, use the hook. + If val is disambiguated to the class `cl`, use its base hook. - If val is an instance of a subclass, dispatch on its exact runtime type. + If val is disambiguated to a subclass, dispatch on its exact runtime + type. """ - if val.__class__ is _cl: - return _base_hook(val) - return _c.unstructure(val, unstructure_as=val.__class__) - - unstruct_predicate = cls_is_cl - else: - if subclass_union is not None: - union_strategy(subclass_union, converter) - struct_hook = converter._union_struct_registry[subclass_union] - else: - struct_hook = None - - if i == 0 and subclass_union is not None: - if subclass_union is not None: - union_classes = get_args(subclass_union) - else: - union_classes = () - - def cls_is_in_union(cls, _union_classes=union_classes): - return cls in _union_classes - - unstruct_hook = converter._unstructure_func.dispatch(subclass_union) - unstruct_predicate = cls_is_in_union - else: - unstruct_hook = None - unstruct_predicate = None + dis_cl = _dis_fn(val) + if dis_cl is _cl: + return _base_hook(val, _cl) + return _c.structure(val, dis_cl) + + def unstruct_hook( + val: parent_subclass_tree[0], + _c=converter, + _cl=cl, + _base_hook=base_unstruct_hook, + ) -> Dict: + """ + If val is an instance of the class `cl`, use the hook. + + If val is an instance of a subclass, dispatch on its exact runtime type. + """ + if val.__class__ is _cl: + return _base_hook(val) + return _c.unstructure(val, unstructure_as=val.__class__) # This needs to use function dispatch, using singledispatch will again # match A and all subclasses, which is not what we want. - if unstruct_hook is not None: - converter.register_unstructure_hook_func(unstruct_predicate, unstruct_hook) + converter.register_structure_hook_func(cls_is_cl, struct_hook) + converter.register_unstructure_hook_func(cls_is_cl, unstruct_hook) + + +def _include_subclasses_with_union_strategy( + converter: Converter, + union_classes: Tuple[Type], + union_strategy: Callable[[Any, BaseConverter], Any], + overrides: Dict[str, AttributeOverride], +): + parent_classes = [cl for cl in union_classes if _has_subclasses(cl, union_classes)] + if not parent_classes: + return - if struct_hook is not None: - converter.register_structure_hook_func(cls_is_cl, struct_hook) + for cl in union_classes: + + def cls_is_cl(cls, _cl=cl): + return cls is _cl + + converter.register_structure_hook_func( + cls_is_cl, make_dict_structure_fn(cl, converter, **overrides) + ) + converter.register_unstructure_hook_func( + cls_is_cl, make_dict_unstructure_fn(cl, converter, **overrides) + ) + + for cl in parent_classes: + subclass_union = _get_union_type(cl, union_classes) + sub_union_classes = get_args(subclass_union) + union_strategy(subclass_union, converter) + struct_hook = converter._union_struct_registry[subclass_union] + unstruct_hook = converter._unstructure_func.dispatch(subclass_union) + + def cls_is_cl(cls, _cl=cl): + return cls is _cl + + def cls_is_in_union(cls, _union_classes=sub_union_classes): + return cls in _union_classes + + # This needs to use function dispatch, using singledispatch will again + # match A and all subclasses, which is not what we want. + converter.register_structure_hook_func(cls_is_cl, struct_hook) + converter.register_unstructure_hook_func(cls_is_in_union, unstruct_hook) From d2654daff1778652c9551fa92cec50a612f36f34 Mon Sep 17 00:00:00 2001 From: Matthieu Melot Date: Fri, 6 Jan 2023 15:27:31 -0500 Subject: [PATCH 13/17] circular reference test is also done with tagged union Unfortunately it does not work... --- tests/strategies/test_include_subclasses.py | 25 +++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/tests/strategies/test_include_subclasses.py b/tests/strategies/test_include_subclasses.py index a76c3ea1..114265c0 100644 --- a/tests/strategies/test_include_subclasses.py +++ b/tests/strategies/test_include_subclasses.py @@ -135,9 +135,11 @@ def conv_w_subclasses(request): c = Converter() if request.param == "with-subclasses": include_subclasses(Parent, c) + include_subclasses(CircularA, c) elif request.param == "with-subclasses-and-tagged-union": union_strategy = partial(configure_tagged_union, tag_name="type_name") include_subclasses(Parent, c, union_strategy=union_strategy) + include_subclasses(CircularA, c, union_strategy=union_strategy) return c, request.param @@ -185,13 +187,28 @@ def test_structure_as_union(): assert res == [Child1(1, 2)] -def test_circular_reference(): - c = Converter() - include_subclasses(CircularA, c) +def test_circular_reference(conv_w_subclasses): + c, included_subclasses_param = conv_w_subclasses + struct = CircularB(a=1, other=[CircularB(a=2, other=[], b=3)], b=4) - unstruct = dict(a=1, other=[dict(a=2, other=[], b=3)], b=4) + unstruct = dict( + a=1, + other=[dict(a=2, other=[], b=3, type_name="CircularB")], + b=4, + type_name="CircularB", + ) + + if included_subclasses_param != "with-subclasses-and-tagged-union": + unstruct = _remove_type_name(unstruct) + + if included_subclasses_param == "wo-subclasses": + # We already now that it will fail + return res = c.unstructure(struct) + if "wo-subclasses" or "tagged-union" in included_subclasses_param: + # TODO: tagged-union should work here, but it does not yet. + pytest.xfail("Cannot succeed if include_subclasses strategy is not used") assert res == unstruct res = c.unstructure(struct, CircularA) From f2b8f3d546188baaa2792c0152b86bd649a73ff7 Mon Sep 17 00:00:00 2001 From: Tin Tvrtkovic Date: Wed, 8 Mar 2023 02:38:15 +0100 Subject: [PATCH 14/17] Tweak the include_subclasses strategy --- src/cattrs/strategies/_subclasses.py | 90 +++++++++++++++------ tests/strategies/test_include_subclasses.py | 47 +++-------- 2 files changed, 75 insertions(+), 62 deletions(-) diff --git a/src/cattrs/strategies/_subclasses.py b/src/cattrs/strategies/_subclasses.py index 6bad17b9..0c1046e5 100644 --- a/src/cattrs/strategies/_subclasses.py +++ b/src/cattrs/strategies/_subclasses.py @@ -1,9 +1,14 @@ """Strategies for customizing subclass behaviors.""" from gc import collect -from typing import Dict, Optional, Tuple, Type, Union, List, Callable, Any, get_args +from typing import Any, Callable, Dict, List, Optional, Tuple, Type, Union, get_args -from ..converters import Converter, BaseConverter -from ..gen import AttributeOverride, make_dict_structure_fn, make_dict_unstructure_fn +from ..converters import BaseConverter, Converter +from ..gen import ( + AttributeOverride, + _already_generating, + make_dict_structure_fn, + make_dict_unstructure_fn, +) def _make_subclasses_tree(cl: Type) -> List[Type]: @@ -12,7 +17,8 @@ def _make_subclasses_tree(cl: Type) -> List[Type]: ] -def _has_subclasses(cl: Type, given_subclasses: Tuple[Type]): +def _has_subclasses(cl: Type, given_subclasses: Tuple[Type, ...]) -> bool: + """Whether the given class has subclasses from `given_subclasses`.""" actual = set(cl.__subclasses__()) given = set(given_subclasses) return bool(actual & given) @@ -31,7 +37,7 @@ def _get_union_type(cl: Type, given_subclasses_tree: Tuple[Type]) -> Optional[Ty def include_subclasses( cl: Type, converter: Converter, - subclasses: Optional[Tuple[Type]] = None, + subclasses: Optional[Tuple[Type, ...]] = None, union_strategy: Optional[Callable[[Any, BaseConverter], Any]] = None, overrides: Optional[Dict[str, AttributeOverride]] = None, ) -> None: @@ -139,40 +145,76 @@ def unstruct_hook( def _include_subclasses_with_union_strategy( converter: Converter, - union_classes: Tuple[Type], + union_classes: Tuple[Type, ...], union_strategy: Callable[[Any, BaseConverter], Any], overrides: Dict[str, AttributeOverride], ): + """ + This function is tricky because we're dealing with what is essentially a circular reference. + + We need to generate a structure hook for a class that is both: + * specific for that particular class and its own fields + * but should handle specific functions for all its descendants too + + Hence the dance with registering below. + """ + parent_classes = [cl for cl in union_classes if _has_subclasses(cl, union_classes)] if not parent_classes: return + original_unstruct_hooks = {} + original_struct_hooks = {} for cl in union_classes: + # In the first pass, every class gets its own unstructure function according to + # the overrides. + # We just generate the hooks, and do not register them. This allows us to manipulate + # the _already_generating set to force runtime dispatch. + _already_generating.working_set = set(union_classes) - {cl} + try: + unstruct_hook = make_dict_unstructure_fn(cl, converter, **overrides) + struct_hook = make_dict_structure_fn(cl, converter, **overrides) + finally: + _already_generating.working_set = set() + original_unstruct_hooks[cl] = unstruct_hook + original_struct_hooks[cl] = struct_hook + + # Now that's done, we can register all the hooks and generate the + # union handler. The union handler needs them. + final_union = Union[union_classes] # type: ignore + + for cl, hook in original_unstruct_hooks.items(): def cls_is_cl(cls, _cl=cl): return cls is _cl - converter.register_structure_hook_func( - cls_is_cl, make_dict_structure_fn(cl, converter, **overrides) - ) - converter.register_unstructure_hook_func( - cls_is_cl, make_dict_unstructure_fn(cl, converter, **overrides) - ) + converter.register_unstructure_hook_func(cls_is_cl, hook) - for cl in parent_classes: - subclass_union = _get_union_type(cl, union_classes) - sub_union_classes = get_args(subclass_union) - union_strategy(subclass_union, converter) - struct_hook = converter._union_struct_registry[subclass_union] - unstruct_hook = converter._unstructure_func.dispatch(subclass_union) + for cl, hook in original_struct_hooks.items(): def cls_is_cl(cls, _cl=cl): return cls is _cl - def cls_is_in_union(cls, _union_classes=sub_union_classes): - return cls in _union_classes + converter.register_structure_hook_func(cls_is_cl, hook) - # This needs to use function dispatch, using singledispatch will again - # match A and all subclasses, which is not what we want. - converter.register_structure_hook_func(cls_is_cl, struct_hook) - converter.register_unstructure_hook_func(cls_is_in_union, unstruct_hook) + union_strategy(final_union, converter) + unstruct_hook = converter._unstructure_func.dispatch(final_union) + struct_hook = converter._structure_func.dispatch(final_union) + + for cl in union_classes: + # In the second pass, we overwrite the hooks with the union hook. + + def cls_is_cl(cls, _cl=cl): + return cls is _cl + + converter.register_unstructure_hook_func(cls_is_cl, unstruct_hook) + subclasses = tuple([c for c in union_classes if issubclass(c, cl)]) + if len(subclasses) > 1: + u = Union[subclasses] # type: ignore + union_strategy(u, converter) + struct_hook = converter._structure_func.dispatch(u) + + def sh(payload: dict, _, _u=u, _s=struct_hook) -> cl: + return _s(payload, _u) + + converter.register_structure_hook_func(cls_is_cl, sh) diff --git a/tests/strategies/test_include_subclasses.py b/tests/strategies/test_include_subclasses.py index 114265c0..febdea92 100644 --- a/tests/strategies/test_include_subclasses.py +++ b/tests/strategies/test_include_subclasses.py @@ -1,13 +1,13 @@ import typing -from functools import partial from copy import deepcopy +from functools import partial +from typing import Tuple import attr import pytest from cattrs import Converter, override from cattrs.errors import ClassValidationError -from cattrs.strategies._subclasses import _make_subclasses_tree from cattrs.strategies import configure_tagged_union, include_subclasses @@ -148,8 +148,8 @@ def conv_w_subclasses(request): "struct_unstruct", IDS_TO_STRUCT_UNSTRUCT.values(), ids=IDS_TO_STRUCT_UNSTRUCT ) def test_structuring_with_inheritance( - conv_w_subclasses: typing.Tuple[Converter, bool], struct_unstruct -): + conv_w_subclasses: Tuple[Converter, bool], struct_unstruct +) -> None: structured, unstructured = struct_unstruct converter, included_subclasses_param = conv_w_subclasses @@ -176,7 +176,7 @@ def test_structuring_with_inheritance( if structured.__class__ in {Parent, Child1, Child2}: with pytest.raises(ClassValidationError): - converter.structure(unstructured, GrandChild) + _ = converter.structure(unstructured, GrandChild) def test_structure_as_union(): @@ -201,14 +201,11 @@ def test_circular_reference(conv_w_subclasses): if included_subclasses_param != "with-subclasses-and-tagged-union": unstruct = _remove_type_name(unstruct) - if included_subclasses_param == "wo-subclasses": - # We already now that it will fail - return + if "wo-subclasses" in included_subclasses_param: + pytest.xfail("Cannot succeed if include_subclasses strategy is not used") res = c.unstructure(struct) - if "wo-subclasses" or "tagged-union" in included_subclasses_param: - # TODO: tagged-union should work here, but it does not yet. - pytest.xfail("Cannot succeed if include_subclasses strategy is not used") + assert res == unstruct res = c.unstructure(struct, CircularA) @@ -222,7 +219,7 @@ def test_circular_reference(conv_w_subclasses): "struct_unstruct", IDS_TO_STRUCT_UNSTRUCT.values(), ids=IDS_TO_STRUCT_UNSTRUCT ) def test_unstructuring_with_inheritance( - conv_w_subclasses: typing.Tuple[Converter, bool], struct_unstruct + conv_w_subclasses: Tuple[Converter, bool], struct_unstruct ): structured, unstructured = struct_unstruct converter, included_subclasses_param = conv_w_subclasses @@ -326,29 +323,3 @@ def test_overrides(with_union_strategy: bool, struct_unstruct: str): assert c.unstructure(structured) == unstructured assert c.structure(unstructured, Parent) == structured assert c.structure(unstructured, structured.__class__) == structured - - -def test_class_tree_generator(): - class P: - pass - - class C1(P): - pass - - class C2(P): - pass - - class GC1(C2): - pass - - class GC2(C2): - pass - - tree_c1 = _make_subclasses_tree(C1) - assert tree_c1 == [C1] - - tree_c2 = _make_subclasses_tree(C2) - assert tree_c2 == [C2, GC1, GC2] - - tree_p = _make_subclasses_tree(P) - assert tree_p == [P, C1, C2, GC1, GC2] From b0e9f1d71bba73e4d2c717a564f2b35148b65d69 Mon Sep 17 00:00:00 2001 From: Tin Tvrtkovic Date: Wed, 8 Mar 2023 02:40:47 +0100 Subject: [PATCH 15/17] Remove unused import --- src/cattrs/strategies/_subclasses.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cattrs/strategies/_subclasses.py b/src/cattrs/strategies/_subclasses.py index 0c1046e5..53fda419 100644 --- a/src/cattrs/strategies/_subclasses.py +++ b/src/cattrs/strategies/_subclasses.py @@ -1,6 +1,6 @@ """Strategies for customizing subclass behaviors.""" from gc import collect -from typing import Any, Callable, Dict, List, Optional, Tuple, Type, Union, get_args +from typing import Any, Callable, Dict, List, Optional, Tuple, Type, Union from ..converters import BaseConverter, Converter from ..gen import ( From cbc774af9d514322a93a05a4819930cf2e6df243 Mon Sep 17 00:00:00 2001 From: Matthieu Melot Date: Sun, 30 Apr 2023 08:28:18 -0400 Subject: [PATCH 16/17] Added include_subclasses documentation --- HISTORY.md | 2 + docs/strategies.md | 133 ++++++++++++++++++++++++++- docs/structuring.md | 4 +- docs/unions.md | 2 +- docs/unstructuring.md | 2 +- src/cattrs/strategies/_subclasses.py | 29 ++++-- 6 files changed, 159 insertions(+), 13 deletions(-) diff --git a/HISTORY.md b/HISTORY.md index cf2fd351..294f12c8 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -16,6 +16,8 @@ - Add optional dependencies for `cattrs.preconf` third-party libraries. ([#337](https://github.com/python-attrs/cattrs/pull/337)) - All preconf converters now allow overriding the default `unstruct_collection_overrides` in `make_converter`. ([#350](https://github.com/python-attrs/cattrs/issues/350) [#353](https://github.com/python-attrs/cattrs/pull/353)) +- Subclasses structuring and unstructuring is now supported via a custom `include_subclasses` strategy. + ([#312](https://github.com/python-attrs/cattrs/pull/312)) ## 22.2.0 (2022-10-03) diff --git a/docs/strategies.md b/docs/strategies.md index 5b4cac76..62a50baa 100644 --- a/docs/strategies.md +++ b/docs/strategies.md @@ -4,7 +4,7 @@ _cattrs_ ships with a number of _strategies_ for customizing un/structuring beha Strategies are prepackaged, high-level patterns for quickly and easily applying complex customizations to a converter. -## Tagged Unions +## Tagged Unions Strategy _Found at {py:func}`cattrs.strategies.configure_tagged_union`._ @@ -119,3 +119,134 @@ The converter is now ready to start structuring Apple notifications. ... print("Can't handle this yet") ``` + +## Include Subclasses Strategy + +_Found at {py:func}`cattrs.strategies.include_subclasses`._ + +The _include subclass_ strategy allows the un/structuring of a base class to an instance of itself or one of its descendants. +Conceptually with this strategy, each time an un/structure operation for the base class is asked, `cattrs` machinery replaces that operation as if the union of the base class and its descendants had been asked instead. + +```{doctest} include_subclass + +>>> from attrs import define +>>> from cattrs.strategies import include_subclasses +>>> from cattrs import Converter + +>>> @define +... class Parent: +... a: int + +>>> @define +... class Child(Parent): +... b: str + +>>> converter = Converter() +>>> include_subclasses(Parent, converter) + +>>> converter.unstructure(Child(a=1, b="foo"), unstructure_as=Parent) +{'a': 1, 'b': 'foo'} + +>>> converter.structure({'a': 1, 'b': 'foo'}, Parent) +Child(a=1, b='foo') +``` + +In the example above, we asked to unstructure then structure a `Child` instance as the `Parent` class and in both cases we correctly obtained back the unstructured and structured versions of the `Child` instance. +If we did not apply the `include_subclasses` strategy, this is what we would have obtained: + +```python +>>> converter_no_subclasses = Converter() + +>>> converter_no_subclasses.unstructure(Child(a=1, b="foo"), unstructure_as=Parent) +{'a': 1} + +>>> converter_no_subclasses.structure({'a': 1, 'b': 'foo'}, Parent) +Parent(a=1) +``` + +Without the application of the strategy, in both unstructure and structure operations, we received a `Parent` instance. + +```{note} +The handling of subclasses is an opt-in feature for two main reasons: +- Performance. While small and probably negligeable in most cases the subclass handling incurs more function calls and has a performance impact. +- Customization. The specific handling of subclasses can be different from one situation to the other. In particular there is not apparent universal good defaults for disambiguating the union type. Consequently The decision is left to the `cattrs` user. +``` + +```{warning} +To work properly, all subclasses must be defined when the `include_subclasses` strategy is applied to a `converter`. If subclasses types are defined later, for instance in the context of a plug-in mechanism using inheritance, then those late defined subclasses will not be part of the subclasses union type and will not be un/structured as expected. +``` + +### Customization + +In the example shown in the previous section, the default options for `include_subclasses` work well because the `Child` class has an attribute that do not exist in the `Parent` class (the `b` attribute). +The automatic union type disambiguation function which is based on finding unique fields for each type of the union works as intended. + +Sometimes, more disambiguation customization is required. +For instance, the unstructuring operation would have failed if `Child` did not have an extra attribute or if a sibling of `Child` had also a `b` attribute. +For those cases, a callable of 2 positional arguments (a union type and a converter) defining a [tagged union strategy](strategies.md#tagged-unions-strategy) can be passed to the `include_subclasses` strategy. +{py:func}`configure_tagged_union()` can be used as-is, but if you want to change its defaults, the [partial](https://docs.python.org/3/library/functools.html#functools.partial) function from the `functools` module in the standard library can come in handy. + +```python + +>>> from functools import partial +>>> from attrs import define +>>> from cattrs.strategies import include_subclasses, configure_tagged_union +>>> from cattrs import Converter + +>>> @define +... class Parent: +... a: int + +>>> @define +... class Child1(Parent): +... b: str + +>>> @define +... class Child2(Parent): +... b: int + +>>> converter = Converter() +>>> union_strategy = partial(configure_tagged_union, tag_name="type_name") +>>> include_subclasses(Parent, converter, union_strategy=union_strategy) + +>>> converter.unstructure(Child1(a=1, b="foo"), unstructure_as=Parent) +{'a': 1, 'b': 'foo', 'type_name': 'Child1'} + +>>> converter.structure({'a': 1, 'b': 1, 'type_name': 'Child2'}, Parent) +Child2(a=1, b=1) +``` + +Other customizations available see are (see {py:func}`include_subclasses()`): +- The exact list of subclasses that should participate to the union with the `subclasses` argument. +- Attribute overrides that permit the customization of attributes un/structuring like renaming an attribute. + +Here is an example involving both customizations: + +```python + +>>> from attrs import define +>>> from cattrs.strategies import include_subclasses +>>> from cattrs import Converter, override + +>>> @define +... class Parent: +... a: int + +>>> @define +... class Child(Parent): +... b: str + +>>> converter = Converter() +>>> include_subclasses( +... Parent, +... converter, +... subclasses=(Parent, Child), +... overrides={"b": override(rename="c")} +... ) + +>>> converter.unstructure(Child(a=1, b="foo"), unstructure_as=Parent) +{'a': 1, 'c': 'foo'} + +>>> converter.structure({'a': 1, 'c': 'foo'}, Parent) +Child(a=1, b='foo') +``` \ No newline at end of file diff --git a/docs/structuring.md b/docs/structuring.md index 09200309..5d9254c7 100644 --- a/docs/structuring.md +++ b/docs/structuring.md @@ -280,7 +280,7 @@ the first time an appropriate union is structured. To support arbitrary unions, register a custom structuring hook for the union (see [Registering custom structuring hooks](structuring.md#registering-custom-structuring-hooks)). -Another option is to use a custom tagged union strategy (see [Strategies - Tagged Unions](strategies.md#tagged-unions)). +Another option is to use a custom tagged union strategy (see [Strategies - Tagged Unions](strategies.md#tagged-unions-strategy)). ### `typing.Final` @@ -456,6 +456,8 @@ attributes holding `attrs` classes and dataclasses. B(b=A(a=1)) ``` +Finally, if an `attrs` or `dataclass` class uses inheritance and as such has one or several subclasses, it can be structured automatically to its exact subtype by using the [include subclasses](strategies.md#include-subclasses-strategy) strategy. + ## Registering Custom Structuring Hooks _cattrs_ doesn't know how to structure non-_attrs_ classes by default, diff --git a/docs/unions.md b/docs/unions.md index f72a4b8c..7fdbbc9b 100644 --- a/docs/unions.md +++ b/docs/unions.md @@ -9,7 +9,7 @@ converter customization (since there are many ways of handling unions). ## Unstructuring unions with extra metadata ```{note} -_cattrs_ comes with the [tagged unions strategy](strategies.md#tagged-unions) for handling this exact use-case since version 22.3. +_cattrs_ comes with the [tagged unions strategy](strategies.md#tagged-unions-strategy) for handling this exact use-case since version 23.1. The example below has been left here for educational purposes, but you should prefer the strategy. ``` diff --git a/docs/unstructuring.md b/docs/unstructuring.md index 5e606b2d..474cc82a 100644 --- a/docs/unstructuring.md +++ b/docs/unstructuring.md @@ -161,7 +161,7 @@ _attrs_ classes and dataclasses are supported out of the box. ## Mixing and Matching Strategies -Converters publicly expose two helper metods, {meth}`Converter.unstructure_attrs_asdict() ` +Converters publicly expose two helper methods, {meth}`Converter.unstructure_attrs_asdict() ` and {meth}`Converter.unstructure_attrs_astuple() `. These methods can be used with custom unstructuring hooks to selectively apply one strategy to instances of particular classes. diff --git a/src/cattrs/strategies/_subclasses.py b/src/cattrs/strategies/_subclasses.py index 53fda419..62d9e4af 100644 --- a/src/cattrs/strategies/_subclasses.py +++ b/src/cattrs/strategies/_subclasses.py @@ -42,15 +42,26 @@ def include_subclasses( overrides: Optional[Dict[str, AttributeOverride]] = None, ) -> None: """ - Modify the given converter so that the attrs/dataclass `cl` is un/structured as if - it was a union of itself and all its subclasses that are defined at the time when - this strategy is applied. - - Subclasses are detected using the `__subclasses__` method, or they can be explicitly - provided. - - overrides is a mapping of some or all the parent class field names to attribute - overrides instantiated with :func:`cattrs.gen.override` + Configure the converter so that the attrs/dataclass `cl` is un/structured as if it + was a union of itself and all its subclasses that are defined at the time when this + strategy is applied. + + :param cl: A base `attrs` or `dataclass` class. + :param converter: The `Converter` on which this strategy is applied. Do note that + the strategy does not work for a :class:`cattrs.BaseConverter`. + :param subclasses: A tuple of sublcasses whose ancestor is `cl`. If left as `None`, + subclasses are detected using recursively the `__subclasses__` method of `cl` + and its descendents. + :param union_strategy: A callable of two arguments passed by position + (`subclass_union`, `converter`) that defines the union strategy to use to + disambiguate the subclasses union. If `None` (the default), the automatic unique + field disambiguation is used which means that every single subclass + participating in the union must have an attribute name that does not exist in + any other sibling class. + :param overrides: a mapping of `cl` attribute names to overrides (instantiated with + :func:`cattrs.gen.override`) to customize un/structuring. + + .. versionadded:: 23.1.0 """ # Due to https://github.com/python-attrs/attrs/issues/1047 collect() From ce287f36cda5ed89ae602f39496877b490549770 Mon Sep 17 00:00:00 2001 From: Matthieu Melot Date: Tue, 16 May 2023 09:53:35 -0400 Subject: [PATCH 17/17] Make newer black happy --- src/cattrs/strategies/_unions.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/cattrs/strategies/_unions.py b/src/cattrs/strategies/_unions.py index 1e017381..51be8e9b 100644 --- a/src/cattrs/strategies/_unions.py +++ b/src/cattrs/strategies/_unions.py @@ -74,7 +74,6 @@ def unstructure_tagged_union( _cl_to_tag=cl_to_tag, _tag_name=tag_name, ) -> Dict: - res = _exact_cl_unstruct_hooks[val.__class__](val) res[_tag_name] = _cl_to_tag[val.__class__] return res