python-attrs / cattrs

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

include_subclasses doesn't work for classes with the same fields #586

Closed karpetrosyan closed 1 month ago

karpetrosyan commented 1 month ago

Code example

Works...

import typing as t

from attrs import define
from cattrs import Converter, structure

@define
class A: ...

@define
class B(A):
    type: t.Literal["b"]

@define
class C(A):
    type: t.Literal["c"]

@define
class L:
    a: B | C

c = Converter()
structure({"a": {"type": "b"}}, L)

But I have many subclasses of A, and I wont modify annotation of L each time, so I tried to use the base class for annotation, like:

import typing as t

from attrs import define
from cattrs import structure, Converter
from cattrs.strategies import include_subclasses

@define
class A: ...

@define
class B(A):
    type: t.Literal["b"]

@define
class C(A):
    type: t.Literal["c"]

include_subclasses(A, Converter())  # to use something like: field: A

But it fails with the following error message

  File "/Users/test/startups/messaging/.venv/lib/python3.12/site-packages/cattrs/disambiguators.py", line 159, in create_default_dis_func
    raise TypeError(f"{cl} has no usable non-default attributes")
TypeError: <class '__main__.C'> has no usable non-default attributes
Tinche commented 1 month ago

Hello!

Looks like in the include_subclasses case, it's trying to actually handle A | B | C, not just B | C. And since A has no type field, it fails.

I guess the simplest solution would be to just add a type field to A too.

import typing as t

from attrs import define

from cattrs import Converter
from cattrs.strategies import include_subclasses

@define
class A:
    type: t.Literal["a"]

@define
class B(A):
    type: t.Literal["b"]

@define
class C(A):
    type: t.Literal["c"]

c = Converter()
include_subclasses(A, c)  # to use something like: field: A
print(c.structure({"type": "b"}, A))

Let me know if this works for you, or we can brainstorm further.

karpetrosyan commented 2 weeks ago

Hey! Thanks for quick response! But it fails the mypy check.

Incompatible types in assignment (expression has type "Literal['b']", base class "A" defined the type as "Literal['a']

Is there any other solution that can be used with the correct typing? Also, it fails when I use an abstract class A, with the message like "only concrete class can be given where "type[A]" is expected" when I am trying to structure some json to the abstract class.

Tinche commented 1 week ago

But it fails the mypy check.

Hm, you're right. Let me think about this a little.

A is supposed to just be an abstract base, you never expect A to be produced, right? Just B and C?

Anyway, what we can do it combine the include_subclasses strategy with the tagged_union strategy. Here's the entire snippet:

import typing as t
from functools import partial
from typing import get_args

from attrs import define, fields

from cattrs import Converter
from cattrs.strategies import configure_tagged_union, include_subclasses

@define
class A:
    pass

@define
class B(A):
    type: t.Literal["b"]

@define
class C(A):
    type: t.Literal["c"]

c = Converter()
include_subclasses(
    A,
    c,
    union_strategy=partial(
        configure_tagged_union,
        tag_name="type",
        tag_generator=lambda cls: (
            get_args(fields(cls).type.type)[0] if hasattr(fields(cls), "type") else ""
        ),
    ),
)
print(c.structure({"type": "b"}, A))
print(c.structure({"type": "c"}, A))

The function to generate the tag name is complex in this example. If we can just say the type field will be the class name, lowercased, the snippet becomes:

from functools import partial

from attrs import define

from cattrs import Converter
from cattrs.strategies import configure_tagged_union, include_subclasses

@define
class A:
    pass

@define
class B(A):
    pass

@define
class C(A):
    pass

c = Converter()
include_subclasses(
    A,
    c,
    union_strategy=partial(
        configure_tagged_union,
        tag_name="type",
        tag_generator=lambda cls: cls.__name__.lower(),
    ),
)
print(c.structure({"type": "b"}, A))
print(c.structure({"type": "c"}, A))

(We don't need the type fields any more.)