Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Union of generics doesn't work #301

Open
Kobzol opened this issue Feb 14, 2023 · 7 comments
Open

Union of generics doesn't work #301

Kobzol opened this issue Feb 14, 2023 · 7 comments
Labels
enhancement New feature or request

Comments

@Kobzol
Copy link
Collaborator

Kobzol commented Feb 14, 2023

Hi! Thanks for this wonderful library (serde FTW!). I was experimenting with it and noticed a strange behavior that happens when generics and union is combined within a dataclass nested field. Here's a minimal reproduction:

@serde
class ClassA:
    a: int


@serde
class ClassB:
    a: int


T = TypeVar("T")


@serde
class Wrapper(Generic[T]):
    inner: T
    meta: str


InnerType = Union[Wrapper[ClassA], Wrapper[ClassB]]


@serde
class Root:
    config: InnerType


wrapper = Wrapper(inner=ClassA(a=1), meta="foo")
print(from_dict(InnerType, to_dict(wrapper)))
# Wrapper(inner=ClassA(a=1), meta='foo') - right

root = Root(wrapper)
print(from_dict(Root, to_dict(root)))  
# Root(config=Wrapper(inner={'a': 1}, meta='foo')) - wrong, ClassA was not deserialized

Basically, when I have a union of generics, and serialize + deserialize it, it deserializes correctly. However, when the same thing is deserialized as a field of another serde dataclass, it doesn't serialize the nested field correctly.

@yukinarit
Copy link
Owner

Hi @Kobzol!

Thank you for reporting a bug, and making awesome OSS! (I am a cargo-pgo user)

Unfortunately, union generics is not yet supported as of now. In your example code, if you pass ClassB in Wrapper pyserde incorrectly deserialize into Wrapper[ClassA]. Let me see if I can implement to support union generics.

wrapper = Wrapper(inner=ClassB(a=1), meta="foo")
print(from_dict(InnerType, to_dict(wrapper)))
# Wrapper(inner=ClassA(a=1), meta='foo') - wrong

@yukinarit yukinarit added the enhancement New feature or request label Feb 15, 2023
@yukinarit yukinarit self-assigned this Feb 15, 2023
@Kobzol
Copy link
Collaborator Author

Kobzol commented Feb 15, 2023

Thanks! I was mostly just fooling around with some data model idea that I had and I noticed that there is a weird behavior mismatch with generics + unions when the field is nested. I didn't realize that even the non-nested use case isn't currently supposed to work :)

@yukinarit
Copy link
Owner

Do you know how to correctly type check Generic instances? Maybe I need to implement my own? 🤔

This raises TypeError: Subscripted generics cannot be used with class and instance checks.

isinstance(GenericFoo[str](10), GenericFoo[int])
isinstance(GenericFoo[int](10), GenericFoo[str])

This always evaluates to true

isinstance(GenericFoo[str](10), GenericFoo[int])
isinstance(GenericFoo[int](10), GenericFoo[str])

@Kobzol
Copy link
Collaborator Author

Kobzol commented Mar 6, 2023

Maybe with this?

>>> typing.get_args(Foo[int])
(<class 'int'>,)

@yukinarit
Copy link
Owner

@Kobzol Thanks,

I find __orig_class__ property to get type parameters from an object. Using __orig_class__, I made is_generic_instance which can correctly check the instance of generic class. But one downside is that this function always requires an object to be instantiated with explicit type parameters (Foo[int](1) works, Foo(i) doesn't work). I am not sure if we can accept this limitation .. 🤔

from typing import Generic, TypeVar, Any, get_origin, get_args
from dataclasses import dataclass

T = TypeVar("T")

@dataclass
class Foo(Generic[T]):
    v: T

def is_generic_instance(o: Any, cls: Any) -> bool:
    return isinstance(o, get_origin(cls)) and get_args(o.__orig_class__) == get_args(cls)

assert not is_generic_instance(Foo[int](1), Foo[str])
assert is_generic_instance(Foo[int](1), Foo[int])

@Kobzol
Copy link
Collaborator Author

Kobzol commented Mar 13, 2023

That's.. unfortunate, indeed. But I'm not sure that we can do better, since Python doesn't really have any type inference for this.

We could maybe do something like this:

  • We know that the class should be generic, as it inherits from Generic[T].
  • We can look at its attributes and find something that should have T (or is somehow derived from it, like List[T]).
  • Check what is the actual type of that attribute for a specific object and derive that e.g. T=int.

But it's incredibly hacky, probably quite difficult and won't work in all cases :)

@yukinarit
Copy link
Owner

Yeah, that's also what I thought.
Maybe I can get the simple generic dataclass with primitive fields working, but not sure for complex ones with nested generics, unions, containers etc.. 😞

@yukinarit yukinarit changed the title Nested combination of generics and union doesn't work Union of generics doesn't work Mar 14, 2023
@yukinarit yukinarit removed their assignment Jun 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: No status
Development

No branches or pull requests

2 participants