ramonhagenaars / jsons

🐍 A Python lib for (de)serializing Python objects to/from JSON
https://jsons.readthedocs.io
MIT License
289 stars 41 forks source link

Dump returns methods from list #139

Open jessekerkhoven opened 3 years ago

jessekerkhoven commented 3 years ago

When dumping a List of a dataclasses it returns method names instead of the data.

This issues is introduced in V1.5.0 (v1.4.2 works as expected). I think commit d2b36fd introduced the issue.

See below for minimal poc to replicate the issue:

@dataclass
class Child:
  name: str

@dataclass
class Person:
  children: List[Child]

test = {"children":[{"name": "Ramon"}]}

jsons.dump(Person(**test))

results in {'children': [{'clear': {}, 'copy': {}, 'fromkeys': {}, 'get': {}, 'items': {}, 'keys': {}, 'pop': {}, 'popitem': {}, 'setdefault': {}, 'update': {}, 'values': {}}]}

ramonhagenaars commented 3 years ago

Hi Jesse, thanks for reporting.

I've looked into your issue and I think that the problem lies with the way you instantiated Person here.

>>> test = {"children":[{"name": "Ramon"}]}
>>> Person(**test)
Person(children=[{'name': 'Ramon'}])

Notice that for this instance children holds a list of dicts rather than a list of Child instances as you presumably intended.

The following works as expected:

>>> jsons.dump(Person(children=[Child(name="Ramon")]))
{'children': [{'name': 'Ramon'}]}
jessekerkhoven commented 3 years ago

Nice catch, I didn't noticed that. This is a issue indeed.

But nevertheless what explains the difference between version 1.4.2 and 1.5.0?

ramonhagenaars commented 3 years ago

As of 1.5.0, there is a dedicated list_serializer that takes the type(...) of an element in a list. It allows for better performance, because it can inspect the generic type of a list once and use it for each element.

To be honest, I would have expected jsons to raise an error here, since it should have tried to dump a dict as if it were a Child (which won't work). It does behave as such when dumping with strict=True though.

I think it's a bit hairy, I'll check into this a bit more and may still come with a patch eventually.