madman-bob / python-dataclasses-serialization

Serialize/deserialize Python dataclasses to various other data formats
MIT License
25 stars 11 forks source link

Pull argument construction out of the try block #18

Closed petered closed 1 year ago

petered commented 1 year ago

If a custom deserialisation function threw a TypeError, it was very hard to find out what was wrong, because the error was caught in a try-except-block. The try was intended to catch when the current class is being loaded with the wrong arguments, but it was also catching (and obfuscating) when a TypeError was raised during construction of the arguments themselves.

Example code that caused this (I would make it a test in this PR but it uses numpy and msg pack so could not be included)

import msgpack_numpy
import numpy as np
from dataclasses_serialization.json import JSONSerializer
from dataclasses_serialization.serializer_base import Serializer, dict_serialization, dict_deserialization, noop_serialization, noop_deserialization, list_deserialization

DataClassWithNumpyPreSerializer = Serializer(
    serialization_functions={
        dict: lambda dct: dict_serialization(dct, key_serialization_func=DataClassWithNumpyPreSerializer.serialize, value_serialization_func=DataClassWithNumpyPreSerializer.serialize),
        list: lambda lst: list(map(JSONSerializer.serialize, lst)),
        (str, int, float, bool, type(None)): noop_serialization,
        np.ndarray: msgpack_numpy.dumps

    },
    deserialization_functions={
        dict: lambda cls, dct: dict_deserialization(cls, dct, key_deserialization_func=DataClassWithNumpyPreSerializer.deserialize, value_deserialization_func=DataClassWithNumpyPreSerializer.deserialize),
        list: lambda cls, lst: list_deserialization(cls, lst, deserialization_func=DataClassWithNumpyPreSerializer.deserialize),
        (str, int, float, bool, type(None)): noop_deserialization,
        np.ndarray: msgpack_numpy.unpackb  # ERROR WAS HERE... loads only takes 1 arg but is called with 2
    }
)

@dataclass
class People:
    name: str
    img: BGRImageArray

@dataclass
class TheBookOfFaces:
    database: Dict[int, People]

def test_serialization_of_numpy_object():
    obj = TheBookOfFaces({
        364226: People(name='Nancy', img=create_random_image((3, 4))),
        32532234: People(name='Abdul', img=create_random_image((3, 4))),
    })
    serobj = DataClassWithNumpyPreSerializer.serialize(obj)
    deser = DataClassWithNumpyPreSerializer.deserialize(TheBookOfFaces, serobj)
    assert np.array_equal(obj.database[364226].img, deser.database[364226].img)

Before the fix, it raised the very unclear

  File "/Users/peter/projects/eagle_eyes/src/eagle_eyes_video_scanner/src/dataclasses-serialization/dataclasses_serialization/serializer_base/dataclasses.py", line 39, in dict_to_dataclass
    raise DeserializationError(
dataclasses_serialization.serializer_base.errors.DeserializationError: Missing one or more required fields to deserialize {'database': {364226: {'name': 'Nancy', 'img': b"\x85\xc4\x02nd\xc3\xc4\x04type\xa3|u1\xc4\x04kind\xc4\x00\xc4\x05shape\x93\x04\x03\x03\xc4\x04data\xc4$\xd0\xbd5\xfdR^\xcc\xb4P3\xe7\x05m\x02\x9a\x16X\xc0\xeb\xf9\xdc\x1b\x9d\xb4\xdd>\x06\xe3\x81\xedF\x08\x9d\xd8X'"}, 32532234: {'name': 'Abdul', 'img': b"\x85\xc4\x02nd\xc3\xc4\x04type\xa3|u1\xc4\x04kind\xc4\x00\xc4\x05shape\x93\x04\x03\x03\xc4\x04data\xc4$\x99'N\x91+\xa0/^\x13S\xc6\x08\x17M\x97\xa4\xe7\xcbv\x19\xc7\xc6O\xa6\xdd\xa0\xf0\xc6\x97R\xde\x90\x8b\x8fF\xbd"}}} as <class '__main__.TheBookOfFaces'>

After the fix, it raised a much more clear message which indicated the problem.

  File "/Users/peter/projects/eagle_eyes/src/eagle_eyes_video_scanner/src/dataclasses-serialization/dataclasses_serialization/serializer_base/serializer.py", line 66, in deserialize
    return deserialization_func(cls, serialized_obj)
TypeError: unpackb() takes 1 positional argument but 2 were given

Which helped make the fix, which was simply to change the line to

np.ndarray: lambda cls, data: msgpack_numpy.unpackb(data) 
madman-bob commented 1 year ago

Merged, and I've simplified your test to remove the numpy dependency.

petered commented 1 year ago

Noice