Attrs: [RFC] Add optional automatic validation and conversion

Created on 26 May 2020  Â·  30Comments  Â·  Source: python-attrs/attrs

Validation and conversion of attribute values is currently only done if the user passes the corresponding arguments to attrib().

In contrast, libs like Pydantic automatically validate and convert all values (in __init__() as well as __setattr__()). It also allows the user to easily pass the most common validators like ge=0 or maxlength=50.

This lets you do fancy stuff like this, which is very useful, e.g., for REST API server models and clients, or generally for loading nested JSON data. Settings/config management is another usecase:

>>> from dateatime import datetime                                                    
>>> from pydantic import BaseModel                                                    
>>>                                                                                   
>>> class Child(BaseModel):                                                           
...     x: int                                                                        
...     y: int                                                                        
...     d: datetime                                                                   
...                                                                                   
>>> class Parent(BaseModel):                                                          
...     name: str                                                                     
...     child: Child                                                                  
...                                                                                   
>>> data = {                                                                          
...     'name': 'spam',                                                               
...     'child': {                                                                    
...         'x': 23,                                                                  
...         'y': '42',  # sic!                                                        
...         'd': '2020-05-04T13:37:00',                                               
...     },                                                                            
... }                                                                                 
>>> Parent(**data)                                                                    
Parent(name='spam', child=Child(x=23, y=42, d=datetime.datetime(2020, 5, 4, 13, 37))) 

While pydantic works quite nicely, it is way slower than attrs and also uses inheritance which is not always what users may want.

I tried implementing a similar functionality but failed. IMHO, there are some changes needed directly within attr’s class-buildng facilities, as the following example illustrates:

from datetime import datetime
from functools import partial

import attr


# Two examples for pre-defined validators for later use
def check_ge(inst, attr, value, bound):
    if value < bound:
        raise ValueError(f'"{attr.name}" is not >= {bound}: {value}')

def check_le(inst, attr, value, bound):
    if value > bound:
        raise ValueError(f'"{attr.name}" is not <= {bound}: {value}')


def validating_class(auto_converters=True, **kwargs):
    # Extension wrapper for "attrs()"
    def wrap(cls):
        # if auto_converters:
        #   Here, I'd have to manually iterate/evaluate all attribs.
        #   This duplicates work in "attr.s()".
        cls = attr.s(auto_attribs=True, frozen=True, slots=True, **kwargs)(cls)
        # if auto_converters:
        #   It is very hard (impossible?) to update "cls" here and add (optional) 
        #   converters to all attributes
        return cls
    return wrap


def field(default, ge=None, le=None, **kwargs):
    # Extension wrapper for "field()"
    v = []
    # This does not work, b/c I cannot access type information here
    # if attr_type is int:
    #     if ge is not None:
    #         v.append(partial(check_ge, bound=ge))
    #     if le is not None:
    #         v.append(partial(check_le, bound=le))

    if 'validator' in kwargs:
        v.append(validator)
    if v:
        kwargs['validator'] = v
    return attr.ib(converter=int, validator=v)


@validating_class()
class Child:
    x: int = field(ge=0, le=100)
    y: int = field(ge=0, le=100)
    d: datetime  # auto-add "field(converter=datertime.fromisoformat)"


@validating_class()
class Parent:
    name: str
    child: Child = # auto-add "field(converter=lambda d: Child(**d))"


data = {'name': 'spam', 'child': {'x': 23, 'y': '42', 'd': '2020-05-04T13:37:00'}}
p = Parent(**data)
print(p)
# Parent(name='spam', child=Child(x=-1, y=0, datetime(2020, 5, 4, 13, 37)))

I already talked about this with @hynek and he asked me create this issue.

I also created an issue for orjson asking to add support for attrs.

Summary (what’s needed)

  • Optional conversions (and validation) of all attributes. Conversion should only happen when necessary (e.g., do not list(value) if value is already a list). This is the most important part.
  • Shortcuts for commonly used validators: attrib(ge=0, le=10).
  • An asjson() function that uses stdlib and provides an encoder that can handle attrs classes and datetimes and similar common types.
  • Explicit orjson support might not be needed when https://github.com/ijl/orjson/issues/92 gets implemented
Feature

Most helpful comment

Yep. That's a property of the mypy plugin. See here for a workaround:
https://github.com/python-attrs/attrs/issues/630#issuecomment-607281253

All 30 comments

Thanks for writing this up

I added a summary to the issue.

If you have no genereal objections against this issue, I can start working on it and see how far I get.

Hi,

in my personal opinion, structuring and conversion should not be part of the class but of something else. So I've created a library called cattrs that implements the something else (called a converter).

For example, in your writeup you're using something that looks like ISO 8601 for the datetime instance, but I think you'd be hard pressed to come to a consensus on this (why not unix timestamps?). Also if you want to use the pendulum DateTime instead, where does that logic live?

So pydantic is not equivalent to attrs, it's equivalent to attrs+cattrs. I'm not saying everyone should use cattrs (and I'm struggling to maintain it myself, if you look at the issues page...), I'm saying if we want this functionality in a blessed way for attrs, it should be another package under the attrs umbrella or another module in the attrs repository, but it should definitely be another component and not part of attrs as it exists now.

Super _busy_ right now, but just to put a rough stake in the ground: I'm 100% interested in making it possible to write a package like pydantic using attrs (aka provide hooks and tools). But structuring and conversion remains out of scope for attrs proper.

I have tried cattrs and it is even faster than pydantic (except if you use dateutil’s parse()), but attrs+cattrs is imho lacking in user experience. You just have to learn and type too much in order to get basic conversion and evaluation done.

I want something, that is super easy to use for most use cases (and that makes the uncommon use cases possible, of course ;-)) and that is backed by attrs.

I don't necessarily want to create a new package. I’d be perfectly happy if attrs implemented the required hooks and if cattrs added another layer of abstraction for a UX like in the example above. :)

I have tried cattrs and it is even faster than pydantic (except if you use dateutil’s parse()),

@sscherfke That got me interested, so I replaced dateutil with ciso8601 in pydantics benchmarks. You can see the results here: https://github.com/samuelcolvin/pydantic/pull/1568.

@sscherfke So cattrs grew out of a set of converters doing what you suggest, actually. I just found that approach ultimately too limiting.

The simplest solution would be to just include a converter like:

def ensure_cls(cls):
    """If the attribute is an instance of cls, pass, else try constructing."""
    def converter(val):
        if isinstance(val, cls):
            return val
        else:
            return cls(**val)
    return converter

in attrs proper.

Your converter (converter=lambda d: Child(**d)) wouldn't work for passing in an existing instance of Child, though. The real type of the field then becomes a Union[Child, Dict[str, Any]] and this complicates Mypy integration. People will want an ensure_enum later too...

@fbergroth

That got me interested, so I replaced dateutil with ciso8601 in pydantics benchmarks. You can see the results here: samuelcolvin/pydantic#1568.

I discovered this while writing an article about attrs, data classes and pydantic which will be published later this week. I just replaced datetuil’s parse with datetime.fromisoformat() :)

@Tinche

So cattrs grew out of a set of converters doing what you suggest, actually. I just found that approach ultimately too limiting.

My idea is to offer a limited set of the most commonly used functionality via a very simple (and possibly limited) API but also provide a more powerful but also more complex API.

The main idea (and problem) is that you do not have to pass a converter to attrib() and that attrs automatically chooses one for you based on the attributes type:

# Just a rough example :)
for attrib in attribs:
    if has(attrib.type):
        c = lambda d: attrib.type(**d)
    elif issubclass(attrib.type, datetime):
        c = datetime.fromisoformat
    else:
        c = attrib.type

    # only apply converter if instance is not already of correct type:
    attrib.converter = lambda i: i if isinstance(i, attr.type) else c(i)

The real type of the field then becomes a Union[Child, Dict[str, Any]] and this complicates Mypy integration.

đŸ€” Yes, this may indeed be a problem. However, I just tested this in pydantic and mypy has no problems with this example:

from pydantic import BaseModel


class Child(BaseModel):
    x: int
    y: int


class Parent(BaseModel):
    name: str
    child: Child


x = Parent(name='spam', child=Child(x=1, y=2))
y = Parent(name='spam', child=Child(x=1, y='2'))  # y is a str!
d = {'name': 'spam', 'child': {'x': 1, 'y': 2}}
z = Parent(**d)  # As when loaded from json
Ă€ = Parent(name=d['name'], child=d['child'])  # Just for fun :)

assert x == y
assert x == z
assert x == À

I need to point out that datetime.fromisoformat() is absolutely not an adequate parser for ISO strings, as the Stdlib docs clearly say. It’s merely the inverse of datetime.isoformat().

I need to point out that datetime.fromisoformat() is absolutely not an adequate parser for ISO strings, as the Stdlib docs clearly say. It’s merely the inverse of datetime.isoformat().

That is (unfortunately) correct. A library like arrow can: arrow.parser.DateTimeParser().parse_iso(text)

I’m aware that fromisoformat() is very limited and that there are other/better parsers (like the ones arrow or pendulum provide). But if the use case is that you only transmit iso-dates generated and parsed by the stdlib, fromisoformat() works perfectly fine. Furhtermore, it is not yet time to discuss a proper parser for my suggested changes. ;-)

This RFC is the result of an article that I have been writing and that I now finally published: https://stefan.sofa-rockers.org/2020/05/29/attrs-dataclasses-pydantic/

Stefan mentioned to me in private he might get it to work using cattrs so this will wait for now.

I’d like to point out one thing tho: pydantic has a great DX but it comes at the price of correctness and flexibility. That is a very valid tradeoff to make and we can see people loving it for that.

However it’s not a trade-off _I_ am willing to make. Correctness, explicitness, and flexibility are more important to me than typing extra characters. We already have a pydantic, so let’s try to be an actual _alternative_ and not try to imitate them.

(JFTR, I’m not saying anyone wanted that; I’m just writing it out explicitly for clarity so please don’t feel anyone attacked :))

I really hope cattrs can rise to the task because it really seems to have its fans!

I needed a quick solution at work that works "good enough" and does not require changes within (c)attrs, so I just created another layer of abstraction above (c)attrs. 🙃

Here is an example:

from datetime import datetime

from mattr import (  # magic attrs – the only thing that mattrs :D
    frozen,  # attrs(auto_attrib, slots, frozen, kw_only)
    field,  # Wrapper around attrib() w/ validation shortcuts
    evolve,  # attrs.evolve()
    fromdict,  # cattrs.structure() with custom converter
    asdict,   # attrs.asdict()
    fromjson,   # cattrs.structure(json.loads()) with custom converter
    asjson,  #  json.dumps(cattrs.unstructure()) with custom converter
)


@frozen()
class Child:
    x: int = field(ge=0)
    y: int = field(ge=0)
    d: datetime


@frozen()
class Parent:
    name: str = field(re='^[a-c]+$', max_length=3)
    child: Child



DATA = {
    'name': 'abc',
    'child': {
        'x': 23,
        'y': '42',  # sic!
        'd': '2020-05-04T13:37:00',
    },
}


p1 = fromdict(DATA, Parent)
print(asdict(p1))

p_json = asjson(p1)
print(p_json)
p2 = fromjson(p_json, Parent)
assert p1 == p2

try:
    p3 = evolve(p2, name='spam')
except ValueError as e:
    print(e)  # ValueError: "name" exceeds the maximum length of 3: 4

The UX is okay, I think. I don’t know how much I (dis)like fromXYZ(data, cls) style. It feels a bit clunky, but looks quite functional and involves less magic than cls(**data). đŸ€·

Validation is done by attrs, conversion is done by cattrs. I still think that it would maybe be nice if we could teach attrs some basic auto-conversion rules based on the attribute type.

Performance for structuring + validation is a bit slower than just attrs+cattrs but I did not optimizie this yet. Dumping is as fast a attrs+cattrs.

@Tinche Do you think we can add something like in the post above to cattrs? This and maybe some documentation improvements would imho improve cattrs apealing and usability a lot and would be relatively easy to do. :)

@sscherfke Doc improvements are always welcome of course.

What from the post above would you add to cattrs exactly? I actually think the approach of you creating your own library wrapping attrs and cattrs, but I just like component-based approaches in general when it comes to software.

You should also probably be aware of this: https://github.com/Tinche/cattrs/blob/5e3f8719c8f611929ec26a5fcf1c4902dbae1445/src/cattr/gen.py#L20. It's a way of generating a tailored unstructuring function for a type-annotated class, it's still a work in progress (working on docs and adding some more features to it atm). The generated function should be as fast as if you had written it by hand (because cattrs writes it for you, and compiles it).

@Tinche I want something that works out-of-the-box with most common datatypes, like Pydantic does. This includes classes, enums and datetimes. I know that it will be hard to draw a line somewhere, but this is not my concern yet. :) (also, users will always be able to extend the defaults)

I am still evaluating which approaches would work at all, how well they would work and how/were they could best be integrated (attrs, cattrs, a new lib).

I just played with attrs to see how hard it would be to get attr auto conversion working, like in the following example:

import attr

@attr.s(auto_attribs=True, auto_convert=True)
class Child:
    x: int = attr.ib()
    y: int = attr.ib(converter=float)


@attr.s(auto_attribs=True, auto_convert=True)
class Parent:
    child: Child
    c: float
    d: float = 3.14


DATA = {
    'c': '1',
    'd': '2',
    'child': {
        'x': '23',
        'y': '42',
    },
}
print(Parent(**DATA))

These are my changes:

diff --git a/src/attr/_make.py b/src/attr/_make.py
index 6c039ab..bffb647 100644
--- a/src/attr/_make.py
+++ b/src/attr/_make.py
@@ -402,7 +402,7 @@ def _collect_base_attrs_broken(cls, taken_attr_names):
     return base_attrs, base_attr_map


-def _transform_attrs(cls, these, auto_attribs, kw_only, collect_by_mro):
+def _transform_attrs(cls, these, auto_attribs, kw_only, collect_by_mro, auto_convert):
     """
     Transform all `_CountingAttr`s on a class into `Attribute`s.

@@ -434,11 +434,25 @@ def _transform_attrs(cls, these, auto_attribs, kw_only, collect_by_mro):
                 continue
             annot_names.add(attr_name)
             a = cd.get(attr_name, NOTHING)
+
+            if auto_convert:
+                def mk_converter(type):
+                    if getattr(type, "__attrs_attrs__", None) is not None:
+                        return lambda v: (type(**v) if isinstance(v, dict) else v)
+                    else:
+                        return type
+                converter = mk_converter(type)
+            else:
+                converter = None
+
             if not isinstance(a, _CountingAttr):
                 if a is NOTHING:
-                    a = attrib()
+                    a = attrib(converter=converter)
                 else:
-                    a = attrib(default=a)
+                    a = attrib(default=a, converter=converter)
+            else:
+                if a.converter is None:
+                    a.converter = converter
             ca_list.append((attr_name, a))

         unannotated = ca_names - annot_names
@@ -552,9 +566,10 @@ class _ClassBuilder(object):
         cache_hash,
         is_exc,
         collect_by_mro,
+        auto_convert,
     ):
         attrs, base_attrs, base_map = _transform_attrs(
-            cls, these, auto_attribs, kw_only, collect_by_mro,
+            cls, these, auto_attribs, kw_only, collect_by_mro, auto_convert
         )

         self._cls = cls
@@ -908,6 +923,7 @@ def attrs(
     auto_detect=False,
     collect_by_mro=False,
     getstate_setstate=None,
+    auto_convert=False,
 ):
     r"""
     A class decorator that adds `dunder
@@ -1151,6 +1167,7 @@ def attrs(
             cache_hash,
             is_exc,
             collect_by_mro,
+            auto_convert,
         )
         if _determine_whether_to_implement(
             cls, repr, auto_detect, ("__repr__",)

It took me a few hours to get this working but in the end, there was not much change needed (for that simple case at least).

Glancing over the code, doesn’t it basically just add converters if they aren’t already present? I know it’s an extra loop, but ISTM this could already be easily achieved outside of attrs by writing an decorator that preparses the class?

In any case, ISTM the most elegant way here would be to add a hook that runs _before_ transform_attrs and whose return value is actually returned? You’d use @attr.s(after_collection=add_auto_converters) and everybody is happy?


For everybody following along once more: I don’t want pydantic _inside_ attrs but I want to give y’all tools to build tools like pydantic – or better. 😉

Glancing over the code, doesn’t it basically just add converters if they aren’t already present?

Yes, exactly. It only adds simple converters if auto_convert is True and if there is not already a converter. These converters work for attrs classes and single-argument types like, int, float, str, or enums.

I know it’s an extra loop, but ISTM this could already be easily achieved outside of attrs by writing an decorator that preparses the class?

I tried that first but that would require too much effort and duplication of code that is already present in attrs’ internals, so IMHO that would not be a good solution.

In any case, ISTM the most elegant way here would be to add a hook that runs _before_ transform_attrs and whose return value is actually returned? You’d use @attr.s(after_collection=add_auto_converters) and everybody is happy?

I can look into it. In any case, a simple implementation of add_auto_converters should than be part of attrs (as well as more common validators (like, ge, le for numbers and max_length for strings).

For everybody following along once more: I don’t want pydantic _inside_ attrs but I want to give y’all tools to build tools like pydantic – or better. 😉

I’m trying to find the sweet spot of very common and easy to implement functionality that should ship with attrs for more convenience and more advance features that may be included in another lib (like cattrs). :)

Another remark: Wrapping attrs() or attrib() with another function (even with functools.partial()) seems to confuse/break mypy. I can not reproduce this now, but I had some issues with this on my work machine.

Yep. That's a property of the mypy plugin. See here for a workaround:
https://github.com/python-attrs/attrs/issues/630#issuecomment-607281253

We should _really_ add the advice from #630 to our docs. I kinda didn't want because I hoped there'd be a better way but it's doubtful that will happen anytime soon – if ever.

@hynek I heard you were looking for a hook. Here is one: 😜

diff --git a/src/attr/__init__.py b/src/attr/__init__.py
index 7c1e643..9e306d3 100644
--- a/src/attr/__init__.py
+++ b/src/attr/__init__.py
@@ -40,6 +40,18 @@ ib = attr = attrib
 dataclass = partial(attrs, auto_attribs=True)  # happy Easter ;)

+# I just put this here for my convenience
+def auto_convert_hook(name, ca, type):
+    if ca.converter is not None:
+        # Do not override explicitly defined converters!
+        return
+    if getattr(type, "__attrs_attrs__", None) is not None:
+        # Attrs classes
+        ca.converter = lambda v: (type(**v) if isinstance(v, dict) else v)
+    else:
+        # Works for bool, int, float, str, Enums, ...
+        ca.converter = type
+
+
 __all__ = [
     "Attribute",
     "Factory",
diff --git a/src/attr/_make.py b/src/attr/_make.py
index 6c039ab..5644dbd 100644
--- a/src/attr/_make.py
+++ b/src/attr/_make.py
@@ -402,7 +402,7 @@ def _collect_base_attrs_broken(cls, taken_attr_names):
     return base_attrs, base_attr_map


-def _transform_attrs(cls, these, auto_attribs, kw_only, collect_by_mro):
+def _transform_attrs(cls, these, auto_attribs, kw_only, collect_by_mro, le_hook):
     """
     Transform all `_CountingAttr`s on a class into `Attribute`s.

@@ -434,6 +434,7 @@ def _transform_attrs(cls, these, auto_attribs, kw_only, collect_by_mro):
                 continue
             annot_names.add(attr_name)
             a = cd.get(attr_name, NOTHING)
+
             if not isinstance(a, _CountingAttr):
                 if a is NOTHING:
                     a = attrib()
@@ -460,6 +461,11 @@ def _transform_attrs(cls, these, auto_attribs, kw_only, collect_by_mro):
             key=lambda e: e[1].counter,
         )

+    # TODO: Merge this block with the next loop to avoid looping twice?
+    if le_hook is not None:
+        for attr_name, ca in ca_list:
+            le_hook(name=attr_name, ca=ca, type=anns.get(attr_name))
+
     own_attrs = [
         Attribute.from_counting_attr(
             name=attr_name, ca=ca, type=anns.get(attr_name)
@@ -552,9 +558,10 @@ class _ClassBuilder(object):
         cache_hash,
         is_exc,
         collect_by_mro,
+        le_hook,
     ):
         attrs, base_attrs, base_map = _transform_attrs(
-            cls, these, auto_attribs, kw_only, collect_by_mro,
+            cls, these, auto_attribs, kw_only, collect_by_mro, le_hook
         )

         self._cls = cls
@@ -908,6 +915,7 @@ def attrs(
     auto_detect=False,
     collect_by_mro=False,
     getstate_setstate=None,
+    le_hook=None,
 ):
     r"""
     A class decorator that adds `dunder
@@ -1151,6 +1159,7 @@ def attrs(
             cache_hash,
             is_exc,
             collect_by_mro,
+            le_hook,
         )
         if _determine_whether_to_implement(
             cls, repr, auto_detect, ("__repr__",)

This has no performance impact compared to using explicit converters for each field. Maybe this is even useful for cattrs!

In my opinion, the basic auto_convert_hook() should be bundled with attrs for convenience (I was even thinking about adding an addional auto_convert=True as an alias to le_hook=auto_convert_hook but I guessed you won’t like it. 😉

The next thing I will look into is if it makes sense to add hooks for asdict() (and astuple()?). This would make it easier to do sth. like this: o = C(**json.loads(data)); json.dumps(asdict(o)).

I could add a similar hook to asdict().

The hook (implementation of hook functions) looks very similar to extended JSON encoders. And if your goal is to just enconde attrs classes with Python’s json encoder, you don’t even need the asdict-hook, but if you want to user other encoders that cannot be extended, the dict-hook is very useful.

The performance of asdict() with a hook is comparable to that of asdict() without a hook but with a custom JSON encoder. Cattrs (un?)structure is faster though, especially with the new gen module. :)

However, I am not yet happy with the usability. E.g., it is not very easy to extend the "default" auto_convert_hook() with converters for other types. You’d have to copy and modify the code. Since the hook is only run once when the classes is beeing created, it’s performance is not that important so I will try to find a more flexible solution.

@hynek would like my changes proposed in #701 to mature for a while in another project.

I’ll integrate them into typed-settings and will use them from there for my other projects.

@hynek What would be your definition of “the auto-converters and validators are mature enough to be added to attrs”? :)

That is an excellent question! I guess “you’ve built something useful and learned/fixed any rough edges”? I think I’ll try to get 20.3 out in the next two weeks so you can start tinkering.

Excelent! Can’t wait for this to happen. 😀

It's helpful for frontend developer to check params if an optional of type validator could raise some exception for the following code:

from attr import attrs

In [2]: @attrs(auto_attribs=True, kw_only=True)
   ...: class T:
   ...:     a: int
   ...:     b: str
   ...:

In [3]: t=T(a='aa', b=2)

In [4]: t
Out[4]: T(a='aa', b=2)

By now, "kw_only" can show missing key, type annotation is not involved in type checking. May be "type_validation" can be added to check wrong field type. I'm not digging so deep into type annotation runtime checking, and Pydantic can do some auto type validation as below:

In [1]: from pydantic import BaseModel

In [2]: class T(BaseModel):
    ...:     a: int
    ...:     b: str
    ...: 

In [3]: t=T(a='aaa', b=1)
---------------------------------------------------------------------------
ValidationError: 1 validation error for T
a
  value is not a valid integer (type=type_error.integer)

Something like this may be added to attrs in the future. A prototype for that feature currently evolves in Typed Settings.

Yea, please check it out and provide feedback, so we can get it right!

Was this page helpful?
0 / 5 - 0 ratings

Related issues

peteroconnor-bc picture peteroconnor-bc  Â·  9Comments

madig picture madig  Â·  3Comments

habnabit picture habnabit  Â·  7Comments

hynek picture hynek  Â·  8Comments

altendky picture altendky  Â·  12Comments