python-attrs / cattrs

Composable custom class converters for attrs, dataclasses and friends.
https://catt.rs
MIT License
829 stars 113 forks source link

attrs 23.1.0 breaks string annotated generics #427

Open diabolo-dan opened 1 year ago

diabolo-dan commented 1 year ago

Description

When working with generic attrs types with cattrs, I updated my version of attrs and it caused failures. Having looked at the changelog in attrs:

https://www.attrs.org/en/stable/changelog.html#id1

This seems likely it was caused by:

attrs.has() and attrs.fields() now handle generic classes correctly. #1079

Changing the behaviour of: (converters.py 976-977)

        attribs = fields(get_origin(cl) or cl if is_generic(cl) else cl)
        if attrs_has(cl) and any(isinstance(a.type, str) for a in attribs):

in which case it probably can't be described as an attrs bug. (But I don't have high confidence in that).

(I confirmed that

            resolve_types(cl)

failed on both versions of attrs, so the difference is somewhere in the if statement)

What I Did

from __future__ import annotations
from typing import TypeVar, Generic

import attrs
import cattrs

T = TypeVar('T')
@attrs.frozen
class A(Generic[T]):
    a: T

converter = cattrs.Converter()
converter.gen_structure_attrs_fromdict(A[int]) # This fails with attrs 23.1.0

#And hence the lower calls won't work

cattrs.structure({'a': 1}, A[int])  # Succeeds with attrs 22.2.0
cattrs.structure({'a': 'one'}, A[int])  # Fails as expected with attrs 22.2.0
Tinche commented 1 year ago

Interesting. It was me who implemented that change in attrs in response to a bug report, so 🙈

Looks like attrs.resolve_types breaks on generic attrs classes, because typing.get_type_hints breaks on generic attrs classes.

I can confirm there's an issue. It took me a while to figure out why it works on old attrs though. I need to talk to @hynek to figure out on which end do we want to fix this though.

In the meantime I think you can resolve the issue by calling attrs.resolve_types(A) by yourself before using the class.

Darkdragon84 commented 3 months ago

is there any news on this issue?