diff --git a/changelog.d/1328.change.md b/changelog.d/1328.change.md new file mode 100644 index 000000000..4013698db --- /dev/null +++ b/changelog.d/1328.change.md @@ -0,0 +1 @@ +`attrs.converters.pipe()` (and it's syntactic sugar of passing a list for `attrs.field()`'s / `attr.ib()`'s *converter* argument) works again when passing `attrs.setters.convert` to *on_setattr* (which is default for `attrs.define`). diff --git a/src/attr/_make.py b/src/attr/_make.py index e251e0c6c..6ff79baa1 100644 --- a/src/attr/_make.py +++ b/src/attr/_make.py @@ -2709,6 +2709,25 @@ def __init__(self, converter, *, takes_self=False, takes_field=False): converter ).get_first_param_type() + self.__call__ = { + (False, False): self._takes_only_value, + (True, False): self._takes_instance, + (False, True): self._takes_field, + (True, True): self._takes_both, + }[self.takes_self, self.takes_field] + + def _takes_only_value(self, value, instance, field): + return self.converter(value) + + def _takes_instance(self, value, instance, field): + return self.converter(value, instance) + + def _takes_field(self, value, instance, field): + return self.converter(value, field) + + def _takes_both(self, value, instance, field): + return self.converter(value, instance, field) + @staticmethod def _get_global_name(attr_name: str) -> str: """ @@ -2915,18 +2934,7 @@ def pipe(*converters): def pipe_converter(val, inst, field): for c in converters: - if isinstance(c, Converter): - val = c.converter( - val, - *{ - (False, False): (), - (True, False): (c.takes_self,), - (False, True): (c.takes_field,), - (True, True): (c.takes_self, c.takes_field), - }[c.takes_self, c.takes_field], - ) - else: - val = c(val) + val = c(val, inst, field) if isinstance(c, Converter) else c(val) return val diff --git a/src/attr/setters.py b/src/attr/setters.py index 3922adbd3..a9ce01698 100644 --- a/src/attr/setters.py +++ b/src/attr/setters.py @@ -4,7 +4,6 @@ Commonly used hooks for on_setattr. """ - from . import _config from .exceptions import FrozenAttributeError @@ -56,14 +55,20 @@ def validate(instance, attrib, new_value): def convert(instance, attrib, new_value): """ - Run *attrib*'s converter -- if it has one -- on *new_value* and return the + Run *attrib*'s converter -- if it has one -- on *new_value* and return the result. .. versionadded:: 20.1.0 """ c = attrib.converter if c: - return c(new_value) + # This can be removed once we drop 3.8 and use attrs.Converter instead. + from ._make import Converter + + if not isinstance(c, Converter): + return c(new_value) + + return c(new_value, instance, attrib) return new_value diff --git a/tests/test_converters.py b/tests/test_converters.py index 4de08d9af..fc812aa01 100644 --- a/tests/test_converters.py +++ b/tests/test_converters.py @@ -28,6 +28,7 @@ def test_pickle(self, takes_self, takes_field): assert c == new_c assert takes_self == new_c.takes_self assert takes_field == new_c.takes_field + assert c.__call__.__name__ == new_c.__call__.__name__ @pytest.mark.parametrize( "scenario", @@ -58,6 +59,38 @@ def test_fmt_converter_call(self, scenario): assert expect == c._fmt_converter_call("le_name", "le_value") + def test_works_as_adapter(self): + """ + Converter instances work as adapters and pass the correct arguments to + the wrapped converter callable. + """ + taken = None + instance = object() + field = object() + + def save_args(*args): + nonlocal taken + taken = args + return args[0] + + Converter(save_args)(42, instance, field) + + assert (42,) == taken + + Converter(save_args, takes_self=True)(42, instance, field) + + assert (42, instance) == taken + + Converter(save_args, takes_field=True)(42, instance, field) + + assert (42, field) == taken + + Converter(save_args, takes_self=True, takes_field=True)( + 42, instance, field + ) + + assert (42, instance, field) == taken + class TestOptional: """ diff --git a/tests/test_setattr.py b/tests/test_setattr.py index c7b90daee..9b7c5e170 100644 --- a/tests/test_setattr.py +++ b/tests/test_setattr.py @@ -109,16 +109,34 @@ def test_pipe(self): used. They can be supplied using the pipe functions or by passing a list to on_setattr. """ + taken = None + + def takes_all(val, instance, attrib): + nonlocal taken + taken = val, instance, attrib + + return val s = [setters.convert, lambda _, __, nv: nv + 1] @attr.s class Piped: - x1 = attr.ib(converter=int, on_setattr=setters.pipe(*s)) + x1 = attr.ib( + converter=[ + attr.Converter( + takes_all, takes_field=True, takes_self=True + ), + int, + ], + on_setattr=setters.pipe(*s), + ) x2 = attr.ib(converter=int, on_setattr=s) p = Piped("41", "22") + assert ("41", p) == taken[:-1] + assert "x1" == taken[-1].name + assert 41 == p.x1 assert 22 == p.x2 @@ -417,3 +435,19 @@ def test_docstring(self): "Method generated by attrs for class WithOnSetAttrHook." == WithOnSetAttrHook.__setattr__.__doc__ ) + + def test_setattr_converter_piped(self): + """ + If a converter is used, it is piped through the on_setattr hooks. + + Regression test for https://github.com/python-attrs/attrs/issues/1327 + """ + + @attr.define # converter on setattr is implied in NG + class C: + x = attr.field(converter=[int]) + + c = C("1") + c.x = "2" + + assert 2 == c.x