Breaks when PEP 563 is active

cattrs version: All versions, as far as I can tell
Python version: Python 3.7
Operating System: Arch Linux

Description

cattr.structure breaks when PEP 563 is activated. This can be done in Python 3.7 with the new from __future__ import annotations.

In short, this defers the evaluation of type annotations. So consider this class:

@attr.s(auto_attribs=True)
class Foo:
    bar: str

Traditionally, __annotations__ looks like this:

>>> Foo.__annotations__
... {'bar': str}

but under PEP 563 (and eventually the default behaviour), it'll look like this:

>>> Foo.__annotations__
... {'bar': 'str'}

This seems to confuse cattr and causes problems. Round-tripping the above class, we now see exceptions like this:

In [16]: cattr.structure(cattr.unstructure(Foo('Hello World')), Foo)
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-16-1bbcc538d3df> in <module>()
----> 1 cattr.structure(cattr.unstructure(Foo('Hello World')), Foo)

~/.local/share/virtualenvs/aastrology-5jK3bc4O/lib/python3.7/site-packages/cattr/converters.py in structure(self, obj, cl)
    176         """Convert unstructured Python data structures to structured data."""
    177         # type: (Any, Type) -> Any
--> 178         return self._structure_func.dispatch(cl)(obj, cl)
    179 
    180     # Classes to Python primitives.

~/.local/share/virtualenvs/aastrology-5jK3bc4O/lib/python3.7/site-packages/cattr/converters.py in structure_attrs_fromdict(self, obj, cl)
    294             except KeyError:
    295                 continue
--> 296             conv_obj[name] = dispatch(type_)(val, type_)
    297 
    298         return cl(**conv_obj)

~/.local/share/virtualenvs/aastrology-5jK3bc4O/lib/python3.7/site-packages/cattr/converters.py in _structure_default(self, obj, cl)
    238             "it.".format(cl)
    239         )
--> 240         raise ValueError(msg)
    241 
    242     def _structure_call(self, obj, cl):

ValueError: Unsupported type: str. Register a structure hook for it.

This fix is probably to apply this logic: https://www.python.org/dev/peps/pep-0563/#resolving-type-hints-at-runtime

Here:

https://github.com/Tinche/cattrs/blob/6e90fd28af8e23c44798b5cbd98e8d5a06eb87ba/src/cattr/converters.py#L287-L296

Which I'm currently digging into.

I have a proof of concept fix, but won't be able to polish it up until tonight.

This fix will hit two birds with one stone, as I guess it also means forward declarations where broken as well, as per #13

Add an evaluation context to support typing.ForwardDef

Today I run in the same bug, took me quite some time to figure out that the error

ValueError: Unsupported type: List[str]. Register a structure hook for it

was caused by

from __future__ import annotations

Any plans to release a new version with the suggested fix?

Oct 2019 and this bug still seems to be present. The proof of concept mentioned above is dated Sep 2018.

Is there any fix pending, or some workaround? The only thing I can think of is remove the from __future__ import annotations, but that defeats the purpose of it, and as mentioned will be the default in Python 4 (a long ways off I know, but ...)

Still happening in 2020. Has anyone figured out a workaround?

This is a disappointing issue. It's my first day using cattrs and I hit this.

This script demonstrates it.

#!/usr/bin/env python
# -*- coding: utf-8 -*-

from typing import List, Optional
from cattr import structure, unstructure
import attr


@attr.s
class Node(object):
    value: int = attr.ib()
    children: Optional[List['Node']] = attr.ib(default=None)


def main():
    g = {
        'value': 1,
        'children': [
            {'value': 3},
            {'value': 4, 'children': [{'value':44}]},
            {'value': 5, 'children': [{'value':55}]}
        ]
    }
    n = structure(g, Node)
    print(n)


if __name__ == "__main__":
    main()

Fixes python-attrs#41: Breaks when PEP 563 is active.

Issue python-attrs#41: Reimplemented string type conversion using mor…

Fixes python-attrs#41: Breaks when PEP 563 is active.

Issue python-attrs#41: Reimplemented string type conversion using mor…

Any update on resolving this issue?

I work around this by explicitly setting the type argument in the attr.ib() statement.

e.g. foo = attr.ib( type=int )

I'm not sure how well that works with MyPy (if at all), but I also add typing comments.

@attr.s( slots=True )
class Point:
    x = attr.ib( type=float, default=0.0 )    # type: float
    y = attr.ib( type=float, default=0.0 )    # type: foat

@attr.s( slots=True )
class Foo:
    count = attr.ib( type=int, default=0 )    # type: int
    average = attr.ib( type=float, init=False )    # type: float
    point = attr.ib( type=Point, factory=Point )    # type: Point

GenConverter solved it for me
Could it be made the default converter, when calling cattr.structure?

Hi all! GenConverter didn't work for me when attr class was also a generic:

from __future__ import annotations
import attr
import cattr
from typing import Generic, Optional, TypeVar

T = TypeVar("T")

@attr.s(auto_attribs=True)
class A(Generic[T]):
  something: int
  
  def foo(self) -> Optional[T]:
      return None

a = cattr.GenConverter().structure({"something": 1}, A)

Raises this because mapping is None

Traceback (most recent call last):
  File "test.py", line 15, in <module>
    a = cattr.GenConverter().structure({"something": "a"}, A)
  File "\.venv\lib\site-packages\cattr\converters.py", line 199, in structure
    return self._structure_func.dispatch(cl)(obj, cl)
  File "\.venv\lib\site-packages\cattr\converters.py", line 531, in gen_structure_attrs_fromdict
    h = make_dict_structure_fn(cl, self, **attrib_overrides)
  File "\.venv\lib\site-packages\cattr\gen.py", line 124, in make_dict_structure_fn
    name_base = getattr(mapping, p.__name__)
AttributeError: 'NoneType' object has no attribute 'T'

I've added an if mapping is not None: before for p in getattr(cl, "__parameters__", ()): to work around that, but I'm curious if this is a correct fix, because it seems that it won't work if I would have a field of a generic type.

Hi all! GenConverter didn't work for me when attr class was also a generic:

from __future__ import annotations
import attr
import cattr
from typing import Generic, Optional, TypeVar

T = TypeVar("T")

@attr.s(auto_attribs=True)
class A(Generic[T]):
  something: int
  
  def foo(self) -> Optional[T]:
      return None

a = cattr.GenConverter().structure({"something": 1}, A)

Raises this because mapping is None

Traceback (most recent call last):
  File "test.py", line 15, in <module>
    a = cattr.GenConverter().structure({"something": "a"}, A)
  File "\.venv\lib\site-packages\cattr\converters.py", line 199, in structure
    return self._structure_func.dispatch(cl)(obj, cl)
  File "\.venv\lib\site-packages\cattr\converters.py", line 531, in gen_structure_attrs_fromdict
    h = make_dict_structure_fn(cl, self, **attrib_overrides)
  File "\.venv\lib\site-packages\cattr\gen.py", line 124, in make_dict_structure_fn
    name_base = getattr(mapping, p.__name__)
AttributeError: 'NoneType' object has no attribute 'T'

I've added an if mapping is not None: before for p in getattr(cl, "__parameters__", ()): to work around that, but I'm curious if this is a correct fix, because it seems that it won't work if I would have a field of a generic type.

You Specified the wrong type in the structure call. I believe cattrs needs to know what all the types are ahead-of-time, in your code you leave the typevar T undefined.

try

a = cattr.GenConverter().structure({"something": 1}, A[int])

@theunkn0wn1

@theunkn0wn1 is correct. You could also use A[Any] if you just want to pass through the value of something.

@tmr232

@tmr232 That's the plan, when we flesh it out a little more.

@Tinche

@Tinche separately, i've been wondering - is there a reason

        if any(isinstance(a.type, str) for a in attribs):
            # PEP 563 annotations - need to be resolved.
            resolve_types(cl)

can't be backported to the original Converter's structure_attrs_from_dict (and the similar code for unstructuring)?

@petergaultney

@petergaultney Due to the differences in how the converters work, this check would have to be done on every structuring call, not once, which I feel is wasteful. Things like these were one of the motivations for the GenConverter in the first place.

I agree that sounds expensive. But wouldn't it be possible to have the original converter maintain a separate cache of resolved attr types, only resolving when the cache is empty?

@petergaultney

@petergaultney Yep, I just didn't consider complicating it further since the GenConverter handles it out of the box. And I do plan to switching to GenConverter when we iron it out a little more anyway.

if i made a PR would you accept it? :)

we make really heavy use of cattrs via typecats, and we're not quite ready to move to GenConverter (though I'm very excited about it), but if the OG Converter supported new-style annotations we'd be able to upgrade to Python 3.9 immediately and stop the endless stream of

from typing import Dict, List, Set, Optional

:D

(but I totally understand if it's not a maintenance penalty you're interested in)

@petergaultney

@petergaultney If it's very important to you I'd consider it. Why can't you move to the GenConverter though? I can help with any issues.

well, typecats is a very opinionated wrapper around attrs+cattrs, and i'm only partway through making it work with GenConverter.

I've taken another look, and here's a wall of text describing some stuff which you will most likely want to skip:

typecats has a need to 'decorate' the behavior of attrs type (un)structuring at runtime based on traits that apply in a general way (as opposed to being registered on a per-type basis). In other words, I want to be able to run code before and after every attrs-related strategy. [1]

With the previous converter, the easiest way I found to do this was to wrap/decorate the attrs-specific structure/unstructure function in MultiStrategyDispatch. It would then dynamically have a chance to run code both before and after the structure/unstructure, but it still got to use the built-in cattrs dispatching mechanism. And this patch really only had to happen once, because we didn't bother registering a bunch of type-specific hooks directly with the converter.

With GenConverter, now there are two or three different paths to structure/unstructure an attrs type - once on initial structure using function dispatch, then possibly with singledispatch, then lastly with direct_dispatch. Instead of decorating 1 place dynamically at patch time, I end up needing to intercept both the dynamic dispatch and the register_cls_list call from GenConverter (so that i can decorate the registered generated code on initial use). I think I've got it mostly working but the code has become exceptionally hard to follow.

It's a big ball of ridiculousness, but it works great for our use cases with DynamoDB, allowing some very specific but dynamic (i.e. non-registered) behavior about the treatment of Literals, attrs classes which inherit from dict, and a few other things.

Resume reading here:

The simplest approach in terms of my code would be if the Converters themselves supported a pre and post hook for all attrs-related dispatch. This would obviously be a cattrs-level change, and since no one else likely has any use for it, I haven't thought much about it.

The next simplest approach, and one requiring no work on cattrs itself, would be to build this directly into my own replacement for MultiStrategyDispatch that intercepts the dispatch but does an "is_attrs_cls" check on every structure type or unstructure object, where True would cause the chosen function to be decorated at runtime on every dispatch. Not very efficient, particularly when we get down to scalar types, since cattrs is already essentially doing this check. And I suppose if I use my own lru_cache the cost probably wouldn't be as bad as I think. Anyway, our primary concerns are developer productivity, not efficiency, so it's almost certainly worth it for us.

All this to say... I think I'm asking for a very narrow patch to the old Converter when what I really want is something more general purpose, and either way I should probably do a bit more work on my end to see if I can reasonably support my own goals using GenConverter as-is.

A final note: thanks for this library. It's undoubtedly saved us hundreds of hours worth of developer time. I consider myself one of its biggest fans. :)

@petergaultney

@petergaultney Thanks for the kind words. Can you open a different ticket for this; I think what you need is actually possible with the GenConverter without much difficulty.

Navigation Menu

Repository navigation

Breaks when PEP 563 is active #41

Description

4 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Breaks when PEP 563 is active #41

Description

Description

Activity

vodik commented on Sep 14, 2018

ceelian commented on Feb 7, 2019

BrendanSimon commented on Oct 10, 2019

ex-nerd commented on Jan 21, 2020

johnwlockwood commented on Feb 14, 2020

johnwlockwood commented on Feb 14, 2020

jacobg commented on Aug 19, 2020

BrendanSimon commented on Aug 19, 2020

4 remaining items

tmr232 commented on Jan 5, 2021

FakeEmperor commented on Feb 16, 2021

theunkn0wn1 commented on Feb 16, 2021

Tinche commented on Feb 16, 2021

Tinche commented on Feb 16, 2021

petergaultney commented on Feb 17, 2021

Tinche commented on Feb 17, 2021

petergaultney commented on Feb 17, 2021

Tinche commented on Feb 17, 2021

petergaultney commented on Feb 17, 2021

petergaultney commented on Feb 17, 2021

Tinche commented on Feb 17, 2021

petergaultney commented on Feb 17, 2021

Tinche commented on Feb 17, 2021

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions