-
-
Notifications
You must be signed in to change notification settings - Fork 114
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
structuring with include_subclasses
does not work when the union type is a direct field of a dataclass.
#430
Comments
Interesting. So what I think is happening is the hook for It's somewhat of a chicken and egg problem. The I wonder if we can be clever and override hooks for children if their type is the parent type. So the hook for |
@Tinche I'm not sure if this is the same problem, but it seems to be an issue with the tagged union. It works fine if I don't use that. Example: >>> import functools
>>> import attrs
>>> import cattrs
>>> import cattrs.strategies
>>> @attrs.define()
... class Base:
... pass
...
>>> @attrs.define()
... class Child(Base):
... field: Base
...
>>> c = cattrs.GenConverter()
>>> cattrs.strategies.include_subclasses(Base, c)
>>> result = c.structure({"field": {"field": {}}}, Base)
>>> result
Child(field=Child(field=Base()))
>>> c = cattrs.GenConverter()
>>> cattrs.strategies.include_subclasses(Base, c, union_strategy=functools.partial(cattrs.strategies.configure_tagged_union,tag_name="exp_type"))
>>> c.unstructure(Child(field=Child(field=Base())))
{'field': {'field': {'exp_type': 'Base'}, 'exp_type': 'Child'}, 'exp_type': 'Child'}
>>> c.structure({'field': {'field': {'exp_type': 'Base'}, 'exp_type': 'Child'}, 'exp_type': 'Child'}, Base)
Child(field=Base()) # Child is lost! I started using cattrs and suddenly one class didn't work. What I had which worked: @attrs.define
class Base:
pass
@attrs.define
class List(Base):
items: List[Base]
@attrs.define
class Child(Base):
left: Base
right: Base List with Child items works fine, but in Child I have to change type of An example with the List-class: >>> c.unstructure(Child(left=List(items=[Base(), Child(left=List(items=[Base()]), right=Base())]),right=Base()))
{'left': {'items': [{'exp_type': 'Base'}, {'left': {'items': [{'exp_type': 'Base'}], 'exp_type': 'List'}, 'right': {'exp_type': 'Base'}, 'exp_type': 'Child'}], 'exp_type': 'List'}, 'right': {'exp_type': 'Base'}, 'exp_type': 'Child'}
>>> d = c.unstructure(Child(left=List(items=[Base(), Child(left=List(items=[Base()]), right=Base())]),right=Base()))
>>> c.structure(d, Base)
Child(left=Base(), right=Base()) Subclasses without tagged union doesn't work for List because it complains about not having any unique fields, which I find strange as well since it clearly has the unique field |
@aha79 @Tinche Here's my ugly workaround which let's me skip the workaround using Optional using the example in my previous comment. >>> import functools
>>> from typing import Dict, Type
>>> import attrs
>>> import cattrs
>>> import cattrs.gen
>>> import cattrs.strategies
>>> @attrs.define()
... class Base:
... tag_to_cls: Dict[str, Type] = {}
... def __init_subclass__(cls, **kwargs):
... super().__init_subclass__(**kwargs)
... cls.tag_to_cls.update({cls.__name__: cls})
...
>>> @attrs.define()
... class Child(Base):
... field: Base
...
>>> c = cattrs.GenConverter()
>>> cattrs.strategies.include_subclasses(Base, c, union_strategy=functools.partial(cattrs.strategies.configure_tagged_union,tag_name="exp_type"))
>>> c.structure({'field': {'field': {'exp_type': 'Base'}, 'exp_type': 'Child'}, 'exp_type': 'Child'}, Base)
Child(field=Base()) # same as in my previous comment, child is lost
>>> def structure_hook(c, o, cl): # cl = Base
... cl = Base.tag_to_cls.get(o["exp_type"], cl) # default to Base
... struct = cattrs.gen.make_dict_structure_fn(cl, c)
... return struct(o, cl)
>>> c.register_structure_hook(Base, functools.partial(structure_hook, c))
>>> c.structure({'field': {'field': {'exp_type': 'Base'}, 'exp_type': 'Child'}, 'exp_type': 'Child'}, Base)
Child(field=Child(field=Base())) |
@anderssonjohan I spent some time debugging your example (phew, these are kind of hard to debug tbh). I think it's the same issue. The reason it works if you change the field type to an |
Looks like this was fixed by a different issue. I've added a unit test for the OP case just the same. |
Description
structuring with
include_subclasses
does not work properly when the union type is a direct field of a dataclass. When it is indirectly used in the type hint of a field it works.What I Did
Now if we (incorrectly) change
A
toOptional[A]
inDerived
it does indeed work. Using Optional[A] instead of A does not harm serialization, as None does never occur. It interferes with static type checking though so it is not a good workaround.The text was updated successfully, but these errors were encountered: