Repository: python-attrs/attrs

Problem statement:
Clarification - Converters vs on_setattr / Convert automatically when setting 
attrs version: 21.2.0

Using the documentation [on Converters](https://www.attrs.org/en/stable/init.html#converters) it was not immediately clear that converters only work on instantiation of a new attrs-enhanced class. (Or I totally missed something.) 

1. Is this the intended function? That the type conversion only happens on class instantiation and not when setting an attribute? (e.g. ```o.x = "2" ``` in the example in the documentation)
    * If yes: Perhaps the documentation could be clarified to explain this and also provide a clear example of using ```on_setattr``` to also run the converter
    * If no: Perhaps there's a bug because setting an attribute when a converter is specified does not work

And further: from a logical design standpoint, it seems that if you're specifying a converter to be run at class instantiation [which means *all* values on *new* instances are run through that converter....] what circumstances would you *not* want that to run when setting attributes? Part of the purpose of converters is to ensure consistent typing and normalization (from the documentation: _"Finally, sometimes you may want to normalize the values coming in. For that attrs comes with converters."_). Given that, it seems strange that you could break that normalization by simply setting an attribute.

TL;DR: Converters only run on class instantiation. Shouldn't they be run when setting an attribute as well?


Hints:
Marking as documentation because `on_setattr` seems to lack narrative docs, and running converters and validators on set as well as init time is probably a common enough use case to warrant some text.
I'm not sure where to put narrative information…should we open a new chapter for it?
I just ran into this behavior that validators are run on instantiation only. 
Please find time to add this to https://www.attrs.org/en/stable/examples.html#validators as a warning. Its behavior is fine as it is as long as its documented. 
As far as documenting on_setattr is concerned doing it bellow validators on same page would be nice. 
I have tried to implement it the same way i did validator (by using decorator example) but ended up with TypeError: 'NoneType" object is not callable. In the end i just used standard python way to define setter via decorator. 
Thanks
I have added blurbs in 95b70bd, although I hope that by the next release, the examples will be rewritten using NG APIs.

---

As for the original issue – I'm willing to break b/w compat here since the APIs are new and the impact is limited to few people. I honestly don't remember why I chose to only run validators – maybe because people kept complaining about it and I just…forgot about converters?
@hynek 
I am also complaining ! ;-)
I would definitely need the converters also on_setattr !

```
@attr.define(slots=True)
class MyDataClass(object):
    my_value=attr.ib(default=0, converter=int)

>>> my_data=MyDataClass()
>>> my_data.my_value='1'
>>> assert 1 == my_data.my_value
```
better yesterday then tomorrow ;-),  i just needed to write ugly and slow own converters - 
it would have saved me a lot of effort it that would work.

I just came here to put a question why it is not working - and landed at this thread.

yours sincerely, und Grüße nach Berlin

bitranox, Vienna




Patch:
diff --git a/src/attr/_make.py b/src/attr/_make.py
--- a/src/attr/_make.py
+++ b/src/attr/_make.py
@@ -59,6 +59,8 @@
 # Unique object for unequivocal getattr() defaults.
 _sentinel = object()
 
+_ng_default_on_setattr = setters.pipe(setters.convert, setters.validate)
+
 
 class _Nothing(object):
     """
@@ -722,13 +724,31 @@ def __init__(
             self._cls_dict["__delattr__"] = _frozen_delattrs
 
             self._wrote_own_setattr = True
-        elif on_setattr == setters.validate:
+        elif on_setattr in (
+            _ng_default_on_setattr,
+            setters.validate,
+            setters.convert,
+        ):
+            has_validator = has_converter = False
             for a in attrs:
                 if a.validator is not None:
+                    has_validator = True
+                if a.converter is not None:
+                    has_converter = True
+
+                if has_validator and has_converter:
                     break
-            else:
-                # If class-level on_setattr is set to validating, but there's
-                # no field to validate, pretend like there's no on_setattr.
+            if (
+                (
+                    on_setattr == _ng_default_on_setattr
+                    and not (has_validator or has_converter)
+                )
+                or (on_setattr == setters.validate and not has_validator)
+                or (on_setattr == setters.convert and not has_converter)
+            ):
+                # If class-level on_setattr is set to convert + validate, but
+                # there's no field to convert or validate, pretend like there's
+                # no on_setattr.
                 self._on_setattr = None
 
         if getstate_setstate:
@@ -2123,9 +2143,7 @@ def _make_init(
                 raise ValueError("Frozen classes can't use on_setattr.")
 
             needs_cached_setattr = True
-        elif (
-            has_cls_on_setattr and a.on_setattr is not setters.NO_OP
-        ) or _is_slot_attr(a.name, base_attr_map):
+        elif has_cls_on_setattr and a.on_setattr is not setters.NO_OP:
             needs_cached_setattr = True
 
     unique_filename = _generate_unique_filename(cls, "init")
diff --git a/src/attr/_next_gen.py b/src/attr/_next_gen.py
--- a/src/attr/_next_gen.py
+++ b/src/attr/_next_gen.py
@@ -8,7 +8,13 @@
 from attr.exceptions import UnannotatedAttributeError
 
 from . import setters
-from ._make import NOTHING, _frozen_setattrs, attrib, attrs
+from ._make import (
+    NOTHING,
+    _frozen_setattrs,
+    _ng_default_on_setattr,
+    attrib,
+    attrs,
+)
 
 
 def define(
@@ -35,8 +41,10 @@ def define(
     match_args=True,
 ):
     r"""
-    The only behavioral differences are the handling of the *auto_attribs*
-    option:
+    Define an ``attrs`` class.
+
+    The behavioral differences to `attr.s` are the handling of the
+    *auto_attribs* option:
 
     :param Optional[bool] auto_attribs: If set to `True` or `False`, it behaves
        exactly like `attr.s`. If left `None`, `attr.s` will try to guess:
@@ -46,9 +54,11 @@ def define(
        2. Otherwise it assumes *auto_attribs=False* and tries to collect
           `attr.ib`\ s.
 
-    and that mutable classes (``frozen=False``) validate on ``__setattr__``.
+    and that mutable classes (``frozen=False``) convert and validate on
+    ``__setattr__``.
 
     .. versionadded:: 20.1.0
+    .. versionchanged:: 21.3.0 Converters are also run ``on_setattr``.
     """
 
     def do_it(cls, auto_attribs):
@@ -86,9 +96,9 @@ def wrap(cls):
 
         had_on_setattr = on_setattr not in (None, setters.NO_OP)
 
-        # By default, mutable classes validate on setattr.
+        # By default, mutable classes convert & validate on setattr.
         if frozen is False and on_setattr is None:
-            on_setattr = setters.validate
+            on_setattr = _ng_default_on_setattr
 
         # However, if we subclass a frozen class, we inherit the immutability
         # and disable on_setattr.


Test patch:
diff --git a/tests/test_functional.py b/tests/test_functional.py
--- a/tests/test_functional.py
+++ b/tests/test_functional.py
@@ -17,7 +17,7 @@
 
 import attr
 
-from attr._compat import PY2, TYPE
+from attr._compat import PY2, PY36, TYPE
 from attr._make import NOTHING, Attribute
 from attr.exceptions import FrozenInstanceError
 
@@ -692,8 +692,9 @@ class C(object):
     @pytest.mark.parametrize("slots", [True, False])
     def test_no_setattr_if_validate_without_validators(self, slots):
         """
-        If a class has on_setattr=attr.setters.validate (default in NG APIs)
-        but sets no validators, don't use the (slower) setattr in __init__.
+        If a class has on_setattr=attr.setters.validate (former default in NG
+        APIs) but sets no validators, don't use the (slower) setattr in
+        __init__.
 
         Regression test for #816.
         """
@@ -713,6 +714,58 @@ class D(C):
         assert "self.y = y" in src
         assert object.__setattr__ == D.__setattr__
 
+    @pytest.mark.parametrize("slots", [True, False])
+    def test_no_setattr_if_convert_without_converters(self, slots):
+        """
+        If a class has on_setattr=attr.setters.convert but sets no validators,
+        don't use the (slower) setattr in __init__.
+        """
+
+        @attr.s(on_setattr=attr.setters.convert)
+        class C(object):
+            x = attr.ib()
+
+        @attr.s(on_setattr=attr.setters.convert)
+        class D(C):
+            y = attr.ib()
+
+        src = inspect.getsource(D.__init__)
+
+        assert "setattr" not in src
+        assert "self.x = x" in src
+        assert "self.y = y" in src
+        assert object.__setattr__ == D.__setattr__
+
+    @pytest.mark.skipif(not PY36, reason="NG APIs are 3.6+")
+    @pytest.mark.parametrize("slots", [True, False])
+    def test_no_setattr_with_ng_defaults(self, slots):
+        """
+        If a class has the NG default on_setattr=[convert, validate] but sets
+        no validators or converters, don't use the (slower) setattr in
+        __init__.
+        """
+
+        @attr.define
+        class C(object):
+            x = attr.ib()
+
+        src = inspect.getsource(C.__init__)
+
+        assert "setattr" not in src
+        assert "self.x = x" in src
+        assert object.__setattr__ == C.__setattr__
+
+        @attr.define
+        class D(C):
+            y = attr.ib()
+
+        src = inspect.getsource(D.__init__)
+
+        assert "setattr" not in src
+        assert "self.x = x" in src
+        assert "self.y = y" in src
+        assert object.__setattr__ == D.__setattr__
+
     def test_on_setattr_detect_inherited_validators(self):
         """
         _make_init detects the presence of a validator even if the field is
diff --git a/tests/test_next_gen.py b/tests/test_next_gen.py
--- a/tests/test_next_gen.py
+++ b/tests/test_next_gen.py
@@ -308,3 +308,28 @@ class MyException(Exception):
 
         assert "foo" == ei.value.x
         assert ei.value.__cause__ is None
+
+    def test_converts_and_validates_by_default(self):
+        """
+        If no on_setattr is set, assume setters.convert, setters.validate.
+        """
+
+        @attr.define
+        class C:
+            x: int = attr.field(converter=int)
+
+            @x.validator
+            def _v(self, _, value):
+                if value < 10:
+                    raise ValueError("must be >=10")
+
+        inst = C(10)
+
+        # Converts
+        inst.x = "11"
+
+        assert 11 == inst.x
+
+        # Validates
+        with pytest.raises(ValueError, match="must be >=10"):
+            inst.x = "9"