From 285b6083310501ebed3a7fef5281dda8f8596469 Mon Sep 17 00:00:00 2001 From: Peter Gaultney Date: Sat, 23 Jan 2021 22:10:27 -0600 Subject: [PATCH 1/2] fix MultiStrategyDispatch to work with new GenConverter and attrs inheritance Because the GenConverter specifically handles generating code for attrs classes, and because it registers hooks that don't require function dispatch, singledispatch was preventing subclasses from having code generated for them, because they would trigger the previously-generated base class's structure/unstructure code. This changes MultiStrategyDispatch to allow for a class-based hook to specifically avoid single-dispatch. Since we know that a given attrs class can always have code generated for it, we don't really want to share dispatch across subclasses - like the original Converter, we want to make sure we're structuring each individual attrs class as its own separate type. --- src/cattr/converters.py | 9 ++++++-- src/cattr/multistrategy_dispatch.py | 30 ++++++++++++++++++++------ tests/test_genconverter_inheritance.py | 20 +++++++++++++++++ 3 files changed, 50 insertions(+), 9 deletions(-) create mode 100644 tests/test_genconverter_inheritance.py diff --git a/src/cattr/converters.py b/src/cattr/converters.py index 85cd1d75..d393639d 100644 --- a/src/cattr/converters.py +++ b/src/cattr/converters.py @@ -508,7 +508,9 @@ def unstructure_attrs_asdict(self, obj: Any) -> Dict[str, Any]: omit_if_default=self.omit_if_default, **attrib_overrides ) - self.register_unstructure_hook(obj.__class__, h) + self._unstructure_func.register_cls_list( + [(obj.__class__, h)], no_singledispatch=True + ) return h(obj) def structure_attrs_fromdict( @@ -524,5 +526,8 @@ def structure_attrs_fromdict( if a.type in self.type_overrides } h = make_dict_structure_fn(cl, self, **attrib_overrides) - self.register_structure_hook(cl, h) + self._structure_func.register_cls_list( + [(cl, h)], no_singledispatch=True + ) + # only direct dispatch so that subclasses get separately generated return h(obj, cl) diff --git a/src/cattr/multistrategy_dispatch.py b/src/cattr/multistrategy_dispatch.py index c326663b..6927a59d 100644 --- a/src/cattr/multistrategy_dispatch.py +++ b/src/cattr/multistrategy_dispatch.py @@ -15,16 +15,24 @@ class _DispatchNotFound(object): class MultiStrategyDispatch(object): """ MultiStrategyDispatch uses a - combination of FunctionDispatch and singledispatch. + combination of exact-match dispatch, singledispatch, and FunctionDispatch. - singledispatch is attempted first. If nothing is - registered for singledispatch, or an exception occurs, + Exact match dispatch is attempted first, based on a direct + lookup of the exact class type, if the hook was registered to avoid singledispatch. + singledispatch is attempted next - it will handle subclasses of base classes using MRO + If nothing is registered for singledispatch, or an exception occurs, the FunctionDispatch instance is then used. """ - __slots__ = ("_function_dispatch", "_single_dispatch", "dispatch") + __slots__ = ( + "_direct_dispatch", + "_function_dispatch", + "_single_dispatch", + "dispatch", + ) def __init__(self, fallback_func): + self._direct_dispatch = dict() self._function_dispatch = FunctionDispatch() self._function_dispatch.register(lambda _: True, fallback_func) self._single_dispatch = singledispatch(_DispatchNotFound) @@ -32,6 +40,9 @@ def __init__(self, fallback_func): def _dispatch(self, cl): try: + direct_dispatch = self._direct_dispatch.get(cl) + if direct_dispatch: + return direct_dispatch dispatch = self._single_dispatch.dispatch(cl) if dispatch is not _DispatchNotFound: return dispatch @@ -39,10 +50,15 @@ def _dispatch(self, cl): pass return self._function_dispatch.dispatch(cl) - def register_cls_list(self, cls_and_handler): - """ register a class to singledispatch """ + def register_cls_list( + self, cls_and_handler, no_singledispatch: bool = False + ): + """ register a class to direct or singledispatch """ for cls, handler in cls_and_handler: - self._single_dispatch.register(cls, handler) + if no_singledispatch: + self._direct_dispatch[cls] = handler + else: + self._single_dispatch.register(cls, handler) self.dispatch.cache_clear() def register_func_list(self, func_and_handler): diff --git a/tests/test_genconverter_inheritance.py b/tests/test_genconverter_inheritance.py new file mode 100644 index 00000000..b38d3694 --- /dev/null +++ b/tests/test_genconverter_inheritance.py @@ -0,0 +1,20 @@ +from cattr.converters import GenConverter +import attr + + +def test_inheritance(): + @attr.s(auto_attribs=True) + class A: + i: int + + @attr.s(auto_attribs=True) + class B(A): + j: int + + converter = GenConverter() + + # succeeds + assert A(1) == converter.structure(dict(i=1), A) + + # fails + assert B(1, 2) == converter.structure(dict(i=1, j=2), B) From a8f28df1f80b739f10069f9d1d948ac633985022 Mon Sep 17 00:00:00 2001 From: Peter Gaultney Date: Sun, 24 Jan 2021 20:55:58 -0600 Subject: [PATCH 2/2] remediation and history entry for genconverter inheritance --- HISTORY.rst | 2 ++ src/cattr/multistrategy_dispatch.py | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/HISTORY.rst b/HISTORY.rst index 6f59b9bc..98835255 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -7,6 +7,8 @@ History * ``converter.unstructure`` now supports an optional parameter, `unstructure_as`, which can be used to unstructure something as a different type. Useful for unions. * Improve support for union un/structuring hooks. Flesh out docs for advanced union handling. (`#115 `_) +* Fix `GenConverter` behavior with inheritance hierarchies of `attrs` classes. + (`#117 `_) (`#116 `_) 1.1.2 (2020-11-29) ------------------ diff --git a/src/cattr/multistrategy_dispatch.py b/src/cattr/multistrategy_dispatch.py index 6927a59d..d211f3bd 100644 --- a/src/cattr/multistrategy_dispatch.py +++ b/src/cattr/multistrategy_dispatch.py @@ -32,7 +32,7 @@ class MultiStrategyDispatch(object): ) def __init__(self, fallback_func): - self._direct_dispatch = dict() + self._direct_dispatch = {} self._function_dispatch = FunctionDispatch() self._function_dispatch.register(lambda _: True, fallback_func) self._single_dispatch = singledispatch(_DispatchNotFound) @@ -41,7 +41,7 @@ def __init__(self, fallback_func): def _dispatch(self, cl): try: direct_dispatch = self._direct_dispatch.get(cl) - if direct_dispatch: + if direct_dispatch is not None: return direct_dispatch dispatch = self._single_dispatch.dispatch(cl) if dispatch is not _DispatchNotFound: