fix: support generic dataclass

[PJCampi]

support generic dataclass

[george-zubrienko]

@matt035343 wdyt about this? Imo this allows you to squeeze lots of problems in as there are no bounds on T, which means it will always be a string/number/bool, what's the point then?

[matt035343]

@george-zubrienko

@matt035343 wdyt about this? Imo this allows you to squeeze lots of problems in as there are no bounds on T, which means it will always be a string/number/bool, what's the point then?

I am unsure what the current behavior is, crash and burn? In that case anything is better.
However, I am interested in seeing some more unit tests for more complex generic types. For example:

• When multiple generic types are accepted
• When the generic type is a list/dict
• etc...

[PJCampi]

The current implementation will not decode the generic dataclass. The dataclass attribute will be assigned the json dict value instead. In my unittest, nested is assigned {"value": "value"}

After my proposed change, whether the decoding works is down to whether T itself is supported by dataclass_json. Currently it works if the materialized type of T is supported (extended types, collections of them, dataclass, etc...). One can also register a global decoder for the TypeVar (f.ex. if T is bound or constrained).

It is not a perfect solution at all but the library's coverage improves a little. I can add more tests to better highlight what works and what doesn't.

[george-zubrienko]

It is not a perfect solution at all but the library's coverage improves a little. I can add more tests to better highlight what works and what doesn't.

Please do that - and request review again when ready :)

[george-zubrienko]

After my proposed change, whether the decoding works is down to whether T itself is supported by dataclass_json

Shouldn't we throw in case it is not supported?

[PJCampi]

Maybe I should describe my change in more details because the name of the PR (and the ticket I opened) is a bit misleading.

It's not about improving the decoding of type variables in a generic dataclass. That feature currently not supported by the library. The library sets the raw json node to a dataclass field of type T regarldess of its bound, constraints or annotation via __cls_getitem__. It never fails:

    @dataclass_json
    @dataclass
    class Foo(Generic[T]):
        bar: T

    @dataclass_json
    @dataclass
    class Bar:
        value: int

    assert Foo[Bar].from_json('{"bar": {"value": 1}}') == Foo({"value": 1})  # Bar is not decoded correctly  

I am trying to align the behaviour of the library when a generic dataclass is used as type annotation to the behavior I just described.

In the case where a generic dataclass (like Foo) is used as part of a type annotation, it is wrapped in an generic wrapper (_GenericAlias in python 3.11).

Consider the following example:

    @dataclass_json
    @dataclass
    class Baz(Generic[T]):
        foo: Foo[T]

On master we silently fail to decode the generic dataclass attribute foo and the json node is assigned to the attribute:

    assert Baz[int].from_json('{"foo": {"bar": 1}}') == Baz({"bar": 1})
    assert Foo[int].from_json('{"bar": 1}') == Foo(1)

After my change we extract the concrete dataclass type from the generic wrapper. The type is now decoded as a dataclass with generic type T. The decoding of foo becomes consistent with the decoding of an instance of Foo:

    assert Baz[int].from_json('{"foo": {"bar": 1}}') == Baz(Foo(1))
    assert Foo[int].from_json('{"bar": 1}') == Foo(1)

Throwing an exception instead of silently incorrectly setting a json node to an attribute of complex type would be a good idea but it's orthogonal to my change, and it would be a breaking change to the behaviour of the library.

I added more test cases now. I did not write any test around type bounds or constraints cause that's not supported by the library anyways.

[george-zubrienko]

I am convinced - @matt035343 any additional input or we release?

[matt035343]

I am convinced - @matt035343 any additional input or we release?

I think it looks good

[george-zubrienko]

To be released Monday :) fyi @PJCampi - and thanks for contributions and thorough explanations!