From 9f53896379b1b8959de5a4572344c4e611d9ade7 Mon Sep 17 00:00:00 2001 From: Nate McMaster Date: Fri, 18 Jun 2021 08:27:35 -0700 Subject: [PATCH] Structure using the value of attr.ib converter, if defined (#139) * Implement support for attrib converters * Update docs per PR feedback * Update make_dict_structure_fn to take bool variable instead of reading _prefer_attrib_converters of converter argument * Remove _passthru * Add type_ attribute to StructureHandlerNotFoundError * Update changelog * Fix broken tests * Fix linting errors * Increase test coverage --- HISTORY.rst | 3 ++ docs/structuring.rst | 54 +++++++++++++++++---- src/cattr/converters.py | 56 ++++++++++++++++------ src/cattr/dispatch.py | 6 ++- src/cattr/errors.py | 9 ++++ src/cattr/gen.py | 49 +++++++++++++++---- tests/test_function_dispatch.py | 6 ++- tests/test_generics.py | 7 ++- tests/test_structure.py | 14 ++++-- tests/test_structure_attrs.py | 84 ++++++++++++++++++++++++++++++++- 10 files changed, 247 insertions(+), 41 deletions(-) create mode 100644 src/cattr/errors.py diff --git a/HISTORY.rst b/HISTORY.rst index ba726fbc..ef6605f6 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -7,6 +7,9 @@ History * Fix ``GenConverter`` mapping structuring for unannotated dicts on Python 3.8. (`#151 `_) * The source code for generated un/structuring functions is stored in the `linecache` cache, which enables more informative stack traces when un/structuring errors happen using the `GenConverter`. This behavior can optionally be disabled to save memory. +* Support using the attr converter callback during structure. + By default, this is a method of last resort, but it can be elevated to the default by setting `prefer_attrib_converters=True` on `Converter` or `GenConverter`. + (`#138 `_) 1.7.1 (2021-05-28) ------------------ diff --git a/docs/structuring.rst b/docs/structuring.rst index 2cdd9507..a14a7e2d 100644 --- a/docs/structuring.rst +++ b/docs/structuring.rst @@ -284,8 +284,8 @@ and their own converters work out of the box. Given a mapping ``d`` and class >>> @attr.s ... class A: - ... a = attr.ib() - ... b = attr.ib(converter=int) + ... a: int = attr.ib() + ... b: int = attr.ib() ... >>> cattr.structure({'a': 1, 'b': '2'}, A) A(a=1, b=2) @@ -298,8 +298,8 @@ Classes like these deconstructed into tuples can be structured using >>> @attr.s ... class A: - ... a = attr.ib() - ... b = attr.ib(converter=int) + ... a: str = attr.ib() + ... b: int = attr.ib() ... >>> cattr.structure_attrs_fromtuple(['string', '2'], A) A(a='string', b=2) @@ -312,8 +312,8 @@ Loading from tuples can be made the default by creating a new ``Converter`` with >>> converter = cattr.Converter(unstruct_strat=cattr.UnstructureStrategy.AS_TUPLE) >>> @attr.s ... class A: - ... a = attr.ib() - ... b = attr.ib(converter=int) + ... a: str = attr.ib() + ... b: int = attr.ib() ... >>> converter.structure(['string', '2'], A) A(a='string', b=2) @@ -321,6 +321,44 @@ Loading from tuples can be made the default by creating a new ``Converter`` with Structuring from tuples can also be made the default for specific classes only; see registering custom structure hooks below. + +Using attribute types and converters +------------------------------------ + +By default, calling "structure" will use hooks registered using ``cattr.register_structure_hook``, +to convert values to the attribute type, and fallback to invoking any converters registered on +attributes with ``attrib``. + +.. doctest:: + + >>> from ipaddress import IPv4Address, ip_address + >>> converter = cattr.Converter() + + # Note: register_structure_hook has not been called, so this will fallback to 'ip_address' + >>> @attr.s + ... class A: + ... a: IPv4Address = attr.ib(converter=ip_address) + + >>> converter.structure({'a': '127.0.0.1'}, A) + A(a=IPv4Address('127.0.0.1')) + +Priority is still given to hooks registered with ``cattr.register_structure_hook``, but this priority +can be inverted by setting ``prefer_attrib_converters`` to ``True``. + +.. doctest:: + + >>> converter = cattr.Converter(prefer_attrib_converters=True) + + >>> converter.register_structure_hook(int, lambda v, t: int(v)) + + >>> @attr.s + ... class A: + ... a: int = attr.ib(converter=lambda v: int(v) + 5) + + >>> converter.structure({'a': '10'}, A) + A(a=15) + + Complex ``attrs`` classes and dataclasses ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -352,7 +390,7 @@ attributes holding ``attrs`` classes and dataclasses. ... >>> @attr.s ... class B: - ... b = attr.ib(type=A) # Legacy syntax. + ... b: A = attr.ib() ... >>> cattr.structure({'b': {'a': '1'}}, B) B(b=A(a=1)) @@ -376,7 +414,7 @@ Here's an example involving a simple, classic (i.e. non-``attrs``) Python class. >>> cattr.structure({'a': 1}, C) Traceback (most recent call last): ... - ValueError: Unsupported type: . Register a structure hook for it. + StructureHandlerNotFoundError: Unsupported type: . Register a structure hook for it. >>> >>> cattr.register_structure_hook(C, lambda d, t: C(**d)) >>> cattr.structure({'a': 1}, C) diff --git a/src/cattr/converters.py b/src/cattr/converters.py index 152c8c26..5ef20434 100644 --- a/src/cattr/converters.py +++ b/src/cattr/converters.py @@ -1,9 +1,11 @@ from collections import Counter from collections.abc import MutableSet as AbcMutableSet +from dataclasses import Field from enum import Enum from functools import lru_cache -from typing import Any, Callable, Dict, Optional, Tuple, Type, TypeVar +from typing import Any, Callable, Dict, Optional, Tuple, Type, TypeVar, Union +from attr import Attribute from attr import has as attrs_has from attr import resolve_types @@ -34,6 +36,7 @@ ) from .disambiguators import create_uniq_field_dis_func from .dispatch import MultiStrategyDispatch +from .errors import StructureHandlerNotFoundError from .gen import ( AttributeOverride, make_dict_structure_fn, @@ -72,14 +75,17 @@ class Converter(object): "_dict_factory", "_union_struct_registry", "_structure_func", + "_prefer_attrib_converters", ) def __init__( self, dict_factory: Callable[[], Any] = dict, unstruct_strat: UnstructureStrategy = UnstructureStrategy.AS_DICT, + prefer_attrib_converters: bool = False, ) -> None: unstruct_strat = UnstructureStrategy(unstruct_strat) + self._prefer_attrib_converters = prefer_attrib_converters # Create a per-instance cache. if unstruct_strat is UnstructureStrategy.AS_DICT: @@ -292,7 +298,11 @@ def _structure_default(self, obj, cl): return obj if is_generic(cl): - fn = make_dict_structure_fn(cl, self) + fn = make_dict_structure_fn( + cl, + self, + _cattrs_prefer_attrib_converters=self._prefer_attrib_converters, + ) self.register_structure_hook(cl, fn) return fn(obj) @@ -301,7 +311,7 @@ def _structure_default(self, obj, cl): "Unsupported type: {0}. Register a structure hook for " "it.".format(cl) ) - raise ValueError(msg) + raise StructureHandlerNotFoundError(msg, type_=cl) @staticmethod def _structure_call(obj, cl): @@ -328,18 +338,34 @@ def structure_attrs_fromtuple( conv_obj = [] # A list of converter parameters. for a, value in zip(fields(cl), obj): # type: ignore # We detect the type by the metadata. - converted = self._structure_attr_from_tuple(a, a.name, value) + converted = self._structure_attribute(a, value) conv_obj.append(converted) return cl(*conv_obj) # type: ignore - def _structure_attr_from_tuple(self, a, _, value): + def _structure_attribute( + self, a: Union[Attribute, Field], value: Any + ) -> Any: """Handle an individual attrs attribute.""" type_ = a.type + attrib_converter = getattr(a, "converter", None) + if self._prefer_attrib_converters and attrib_converter: + # A attrib converter is defined on this attribute, and prefer_attrib_converters is set + # to give these priority over registered structure hooks. So, pass through the raw + # value, which attrs will flow into the converter + return value if type_ is None: # No type metadata. return value - return self._structure_func.dispatch(type_)(value, type_) + + try: + return self._structure_func.dispatch(type_)(value, type_) + except StructureHandlerNotFoundError: + if attrib_converter: + # Return the original value and fallback to using an attrib converter. + return value + else: + raise def structure_attrs_fromdict( self, obj: Mapping[str, Any], cl: Type[T] @@ -348,10 +374,7 @@ def structure_attrs_fromdict( # For public use. conv_obj = {} # Start with a fresh dict, to ignore extra keys. - dispatch = self._structure_func.dispatch for a in fields(cl): # type: ignore - # We detect the type by metadata. - type_ = a.type name = a.name try: @@ -362,9 +385,7 @@ def structure_attrs_fromdict( if name[0] == "_": name = name[1:] - conv_obj[name] = ( - dispatch(type_)(val, type_) if type_ is not None else val - ) + conv_obj[name] = self._structure_attribute(a, val) return cl(**conv_obj) # type: ignore @@ -484,9 +505,10 @@ def _get_dis_func(union): ) if not all(has(get_origin(e) or e) for e in union_types): - raise ValueError( + raise StructureHandlerNotFoundError( "Only unions of attr classes supported " - "currently. Register a loads hook manually." + "currently. Register a loads hook manually.", + type_=union, ) return create_uniq_field_dis_func(*union_types) @@ -509,9 +531,12 @@ def __init__( forbid_extra_keys: bool = False, type_overrides: Mapping[Type, AttributeOverride] = {}, unstruct_collection_overrides: Mapping[Type, Callable] = {}, + prefer_attrib_converters: bool = False, ): super().__init__( - dict_factory=dict_factory, unstruct_strat=unstruct_strat + dict_factory=dict_factory, + unstruct_strat=unstruct_strat, + prefer_attrib_converters=prefer_attrib_converters, ) self.omit_if_default = omit_if_default self.forbid_extra_keys = forbid_extra_keys @@ -662,6 +687,7 @@ def gen_structure_attrs_fromdict(self, cl: Type[T]) -> T: cl, self, _cattrs_forbid_extra_keys=self.forbid_extra_keys, + _cattrs_prefer_attrib_converters=self._prefer_attrib_converters, **attrib_overrides, ) self._structure_func.register_cls_list([(cl, h)], direct=True) diff --git a/src/cattr/dispatch.py b/src/cattr/dispatch.py index 1a3b1023..11aa78c0 100644 --- a/src/cattr/dispatch.py +++ b/src/cattr/dispatch.py @@ -3,6 +3,8 @@ import attr +from .errors import StructureHandlerNotFoundError + @attr.s class _DispatchNotFound: @@ -121,4 +123,6 @@ def dispatch(self, typ): return handler(typ) else: return handler - raise KeyError("unable to find handler for {0}".format(typ)) + raise StructureHandlerNotFoundError( + f"unable to find handler for {typ}", type_=typ + ) diff --git a/src/cattr/errors.py b/src/cattr/errors.py new file mode 100644 index 00000000..616e5ebf --- /dev/null +++ b/src/cattr/errors.py @@ -0,0 +1,9 @@ +from typing import Type + + +class StructureHandlerNotFoundError(Exception): + """Error raised when structuring cannot find a handler for converting inputs into :attr:`type_`.""" + + def __init__(self, message: str, type_: Type) -> None: + super().__init__(message) + self.type_ = type_ diff --git a/src/cattr/gen.py b/src/cattr/gen.py index 04f1c75c..787927fc 100644 --- a/src/cattr/gen.py +++ b/src/cattr/gen.py @@ -1,13 +1,18 @@ +import functools import linecache import re import uuid from dataclasses import is_dataclass -from typing import Any, Optional, Type, TypeVar +from typing import Any, Optional, TYPE_CHECKING, Type, TypeVar import attr from attr import NOTHING, resolve_types from ._compat import adapted_fields, get_args, get_origin, is_bare, is_generic +from .errors import StructureHandlerNotFoundError + +if TYPE_CHECKING: + from cattr.converters import Converter @attr.s(slots=True, frozen=True) @@ -130,9 +135,10 @@ def generate_mapping(cl: Type, old_mapping): def make_dict_structure_fn( cl: Type, - converter, + converter: "Converter", _cattrs_forbid_extra_keys: bool = False, _cattrs_use_linecache: bool = True, + _cattrs_prefer_attrib_converters: bool = False, **kwargs, ): """Generate a specialized dict structuring function for an attrs class.""" @@ -185,11 +191,18 @@ def make_dict_structure_fn( # For each attribute, we try resolving the type here and now. # If a type is manually overwritten, this function should be # regenerated. - if type is not None: + if _cattrs_prefer_attrib_converters and a.converter is not None: + # The attribute has defined its own conversion, so pass + # the original value through without invoking cattr hooks + handler = None + elif type is not None: handler = converter._structure_func.dispatch(type) else: handler = converter.structure + if not _cattrs_prefer_attrib_converters and a.converter is not None: + handler = _fallback_to_passthru(handler) + struct_handler_name = f"structure_{an}" globs[struct_handler_name] = handler @@ -197,14 +210,21 @@ def make_dict_structure_fn( kn = an if override.rename is None else override.rename globs[f"type_{an}"] = type if a.default is NOTHING: - lines.append( - f" '{ian}': {struct_handler_name}(o['{kn}'], type_{an})," - ) + if handler: + lines.append( + f" '{ian}': {struct_handler_name}(o['{kn}'], type_{an})," + ) + else: + lines.append(f" '{ian}': o['{kn}'],") else: post_lines.append(f" if '{kn}' in o:") - post_lines.append( - f" res['{ian}'] = {struct_handler_name}(o['{kn}'], type_{an})" - ) + if handler: + post_lines.append( + f" res['{ian}'] = {struct_handler_name}(o['{kn}'], type_{an})" + ) + else: + post_lines.append(f" res['{ian}'] = o['{kn}']") + lines.append(" }") if _cattrs_forbid_extra_keys: allowed_fields = {a.name for a in attrs} @@ -237,6 +257,17 @@ def make_dict_structure_fn( return globs[fn_name] +def _fallback_to_passthru(func): + @functools.wraps(func) + def invoke(obj, type_): + try: + return func(obj, type_) + except StructureHandlerNotFoundError: + return obj + + return invoke + + def make_iterable_unstructure_fn(cl: Any, converter, unstructure_to=None): """Generate a specialized unstructure function for an iterable.""" handler = converter.unstructure diff --git a/tests/test_function_dispatch.py b/tests/test_function_dispatch.py index 18d9360d..cd629643 100644 --- a/tests/test_function_dispatch.py +++ b/tests/test_function_dispatch.py @@ -1,14 +1,16 @@ import pytest -from cattr.dispatch import FunctionDispatch +from cattr.dispatch import FunctionDispatch, StructureHandlerNotFoundError def test_function_dispatch(): dispatch = FunctionDispatch() - with pytest.raises(KeyError): + with pytest.raises(StructureHandlerNotFoundError) as exc: dispatch.dispatch(float) + assert exc.value.type_ is float + test_func = object() dispatch.register(lambda cls: issubclass(cls, float), test_func) diff --git a/tests/test_generics.py b/tests/test_generics.py index d02b34c0..784fe910 100644 --- a/tests/test_generics.py +++ b/tests/test_generics.py @@ -4,6 +4,7 @@ from attr import asdict, attrs from cattr import Converter, GenConverter +from cattr.errors import StructureHandlerNotFoundError T = TypeVar("T") T2 = TypeVar("T2") @@ -79,11 +80,13 @@ def test_raises_if_no_generic_params_supplied(converter): data = TClass(1, "a") with pytest.raises( - ValueError, + StructureHandlerNotFoundError, match="Unsupported type: ~T. Register a structure hook for it.", - ): + ) as exc: converter.structure(asdict(data), TClass) + assert exc.value.type_ is T + def test_unstructure_generic_attrs(): c = GenConverter() diff --git a/tests/test_structure.py b/tests/test_structure.py index b14f7369..c89b5ea7 100644 --- a/tests/test_structure.py +++ b/tests/test_structure.py @@ -33,6 +33,7 @@ from cattr import Converter from cattr._compat import is_bare, is_union_type from cattr.converters import NoneType +from cattr.errors import StructureHandlerNotFoundError from . import ( dicts_of_primitives, @@ -309,9 +310,11 @@ class Bar(object): converter.register_structure_hook_func(can_handle, handle) assert converter.structure(10, Foo) == "hi" - with raises(ValueError): + with raises(StructureHandlerNotFoundError) as exc: converter.structure(10, Bar) + assert exc.value.type_ is Bar + @given(data(), enums_of_primitives()) def test_structuring_enums(data, enum): @@ -325,11 +328,16 @@ def test_structuring_enums(data, enum): def test_structuring_unsupported(): """Loading unsupported classes should throw.""" converter = Converter() - with raises(ValueError): + with raises(StructureHandlerNotFoundError) as exc: converter.structure(1, Converter) - with raises(ValueError): + + assert exc.value.type_ is Converter + + with raises(StructureHandlerNotFoundError) as exc: converter.structure(1, Union[int, str]) + assert exc.value.type_ is Union[int, str] + def test_subclass_registration_is_honored(): """If a subclass is registered after a superclass, diff --git a/tests/test_structure_attrs.py b/tests/test_structure_attrs.py index bb71518d..0d0a8b8f 100644 --- a/tests/test_structure_attrs.py +++ b/tests/test_structure_attrs.py @@ -1,8 +1,19 @@ """Loading of attrs classes.""" +from ipaddress import IPv4Address, IPv6Address, ip_address from typing import Union +from unittest.mock import Mock import pytest -from attr import NOTHING, Factory, asdict, astuple, define, fields +from attr import ( + NOTHING, + Factory, + asdict, + astuple, + attrib, + define, + fields, + make_class, +) from hypothesis import assume, given from hypothesis.strategies import data, lists, sampled_from @@ -153,3 +164,74 @@ class ClassWithLiteral: assert converter.structure( {"literal_field": 4}, ClassWithLiteral ) == ClassWithLiteral(4) + + +@pytest.mark.parametrize("converter_type", [Converter, GenConverter]) +def test_structure_fallback_to_attrib_converters(converter_type): + attrib_converter = Mock() + attrib_converter.side_effect = lambda val: str(val) + + def called_after_default_converter(val): + if not isinstance(val, int): + raise ValueError( + "The 'int' conversion should have happened first by the built-in hooks" + ) + return 42 + + converter = converter_type() + cl = make_class( + "HasConverter", + { + # non-built-in type with custom converter + "ip": attrib( + type=Union[IPv4Address, IPv6Address], converter=ip_address + ), + # attribute without type + "x": attrib(converter=attrib_converter), + # built-in types converters + "z": attrib(type=int, converter=called_after_default_converter), + }, + ) + + inst = converter.structure(dict(ip="10.0.0.0", x=1, z="3"), cl) + + assert inst.ip == IPv4Address("10.0.0.0") + assert inst.x == "1" + attrib_converter.assert_any_call(1) + assert inst.z == 42 + + +@pytest.mark.parametrize("converter_type", [Converter, GenConverter]) +def test_structure_prefers_attrib_converters(converter_type): + attrib_converter = Mock() + attrib_converter.side_effect = lambda val: str(val) + + converter = converter_type(prefer_attrib_converters=True) + cl = make_class( + "HasConverter", + { + # non-built-in type with custom converter + "ip": attrib( + type=Union[IPv4Address, IPv6Address], converter=ip_address + ), + # attribute without type + "x": attrib(converter=attrib_converter), + # built-in types converters + "y": attrib(type=int, converter=attrib_converter), + # attribute with type and default value + "z": attrib(type=int, converter=attrib_converter, default=5), + }, + ) + + inst = converter.structure(dict(ip="10.0.0.0", x=1, y=3), cl) + + assert inst.ip == IPv4Address("10.0.0.0") + + attrib_converter.assert_any_call(1) + assert inst.x == "1" + + attrib_converter.assert_any_call(3) + assert inst.y == "3" + + attrib_converter.assert_any_call(5) + assert inst.z == "5"