python-attrs / cattrs

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

Unstructuring regression for optional generic types #465

Closed rymndhng closed 9 months ago

rymndhng commented 10 months ago

Description

I upgraded cattr from 22.2.0 -> 23.2.0 and I noticed a difference in behaviour for unstructuring optional generic types.

I've attached a minimally reproducing script that shows the differences in output between the two versions

What I Did

See this code:

import attrs
import cattrs

from typing import Generic, Optional, TypeVar

T = TypeVar("T")

@attrs.define
class A:
    foo: str

@attrs.define
class B(Generic[T]):
    a: T

@attrs.define
class BOptional(Generic[T]):
    a: Optional[T]

@attrs.define
class BOptionalPipe(Generic[T]):
    a: T | None

a = A("bar")

b = B(a)
b_optional = BOptional(a)
b_pipe = BOptionalPipe(a)

print(cattrs.unstructure(b))
print(cattrs.unstructure(b_optional))
print(cattrs.unstructure(b_pipe))

On cattrs 22.2.0, this prints:

{'a': {'foo': 'bar'}}
{'a': {'foo': 'bar'}}
{'a': {'foo': 'bar'}}

On cattrs 23.2.0, this prints:

{'a': {'foo': 'bar'}}
{'a': A(foo='bar')}
{'a': A(foo='bar')}

I noticed that dispatching on a generic type parameter returns the identity function, whereas previously, this would dispatch to the value type. It seems like there's a missing hook for handling typevars with optional.

cattrs.register_unstructure_hook_func(
    typing_inspect.is_typevar, cattrs.unstructure
)
Tinche commented 10 months ago

Hm, confirming. Let me see if I can fix this for 23.2.3.

Tinche commented 10 months ago

Fixed and tests added. I'll release 23.2.3 later today most likely.

rymndhng commented 10 months ago

thanks @Tinche !

Tinche commented 9 months ago

This was fixed, closing now