python-attrs / cattrs

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

Unstructure hook for `NewType` not invoked if `NewType` appears in a union #380

Closed layday closed 1 year ago

layday commented 1 year ago

Description

NewType unstructure hooks are not invoked if the new type appears in a union with other types.

What I Did

from typing import NewType

from attr import define
from cattr import Converter

Foo = NewType('Foo', str)

converter = Converter()
converter.register_unstructure_hook(Foo, lambda v: v.replace("foo", "bar"))

@define
class ModelWithFoo:
    total_foo: Foo
    maybe_foo: Foo | None

print(converter.unstructure(ModelWithFoo(Foo("foo"), Foo("is it a foo?"))))
# Produces: {'total_foo': 'bar', 'maybe_foo': 'is it a foo?'}
Tinche commented 1 year ago

You're right, our optional handling is very old and can be improved. Taking a look at it now.

Tinche commented 1 year ago

This should be fixed on main now, let me know!

layday commented 1 year ago

Yep, works now, thanks.

layday commented 1 year ago

Although, if I replace | None with e.g. | bool, it doesn't - not sure if that's intentional. String subclasses work with other kinds of unions.

Tinche commented 1 year ago

Yeah we don't support any union, just a subset for now. It's tricky to do well.

layday commented 1 year ago

Alright, thanks for clarifying!