jsons icon indicating copy to clipboard operation
jsons copied to clipboard

Optional and Union fields are not handled correctly

Open aquamatthias opened this issue 2 years ago • 2 comments

Thanks for a great helper library. I am not able to use dataclasses in combination with Optional or Union.

Example:

@dataclass
class Foo:
    static: ClassVar[str] = "static"
    a: str
    b: int
    c: Optional[str] = None
    d: Union[str, int] = "test"


def test_roundtrip() -> None:
    foo = Foo("foo", 42, "bar")
    # works
    assert jsons.dump(foo) == {"static": "static", "a": "foo", "b": 42, "c": "bar", "d": "test"}
    # actual {'a': 'foo', 'b': 42}
    assert jsons.dump(foo, strip_class_variables=True) == {"a": "foo", "b": 42, "c": "bar", "d": "test"}

When strip_class_variables is used (which is required in my setup) it does not render Optional or Union types any longer. Any idea how to deal with this?

aquamatthias avatar Jun 30 '22 11:06 aquamatthias

Hi @aquamatthias,

I tested your code and it does indeed not render c and d. What I found however, was that it had nothing to do with Optional or Union, but with there being default values. jsons considers Foo.c and Foo.d as class attributes, because c and d both actually exist on Foo and hold a value:

>>> Foo.d
'test'

Therefore they get skipped when strip_class_variables=True. I'm not so sure what to think of this... technically, jsons is right, because c and d can be accessed on the class. But on a dataclass, it is not unusual to use default values for instance attributes like you do. 🤔

Anyhow, would using strip_attr be of any help in your case?

jsons.dump(foo, strip_attr=("static",))

ramonhagenaars avatar Jul 27 '22 19:07 ramonhagenaars

Hey @ramonhagenaars. Thanks for the explanation - I was unaware of the implication with the default value. I think the workaround is possible for cases where the class variables are known upfront. From a pythonic/library point of view, I still think the test above should work - this is what I would expect without thinking about it.

aquamatthias avatar Aug 15 '22 09:08 aquamatthias