serpy
serpy copied to clipboard
clarkduvall
Add support for deserialization
- Added support for read-only fields
- Renamed
.datato.representation(and added.internal_value) for better clarity - Renamed
.to_valueto.to_representation(and addedto_internal_value) for better clarity
TBD update documentation, fix benchmarks, update benchmarks
Fixes clarkduvall/serpy#1
Coverage remained the same at 100.0% when pulling 6d7fcafc0371eea072859a1ef90fed1f71bf0c84 on maroux:deserialization into 3663aafd10f86b27645fd62a7a00a64f692f4a13 on clarkduvall:master.
quick high level comment:
I like the idea of having a similar API to DRF serializers using .to_representation and .to_internal_value now that both serialization and deserialization is supported. Along with that, I would like to keep .data instead of .representation, since it will be an easier drop in replacement for DRF.
It looks like DRF uses update/create/save for deserialization: http://www.django-rest-framework.org/api-guide/serializers/#saving-instances and Marshmallow uses make_object: http://marshmallow.readthedocs.org/en/latest/quickstart.html#deserializing-to-objects
I'm thinking something like that would be nicer than the _cls class variable. What do you think of a make_object method like Marshmallow?
would be cool to add some benchmarks comparing the deserialization speed to DRF and Marshmallow, but that doesn't have to be in this PR
since it will be an easier drop in replacement for DRF.
Anything that'd make it easy to use as an alternative serializer implementation for REST framework would get a big +1 from me :smile:
@clarkduvall @tomchristie Agreed that interface should be a drop-in replacement of DRF.
How about:
Serializer(instance=obj).data
and
Serializer(data=data).instance (this has to be slightly different because DRF 3.0 changed to explicitly using .create(validated_data). To be fair - Serpy could also do the same .create() does deserialization, but I like the property access pattern instead of method call.
Agreed on the _cls class variable. I wasn't sure what's the best way - perhaps Meta inner class? What do you think? I'm entirely sure about the impact of that on performance - I think it's time I set up the benchmarks :)
@maroux I like the Serializer(instance=obj).data and Serializer(data=data).instance
I think it would be more flexible to have a function instead of just specifying the class for deserialization in case some extra massaging of the data has to be done before returning the instance. So if we had a make_instance function that takes the serialized data you could make the .instance property something like:
def make_instance(self, data):
return SomeClass(**data)
@property
def instance(self):
if self._instance is None:
self._instance = self.make_instance(self.to_internal_value(self._data))
return self._instance
by default make_instance could just be a noop
@clarkduvall So the problem with make_instance approach is two-fold: I think Serpy should do instance -> dict, and dict -> instance and since the default serializer is capable of taking in an object, I think it should also be able to create an object of arbitrary type.
How about this:
Serializer(instance=obj).data
and
Serializer(data=data).create(klass)
I'm not sure if there is value is setting a default value of klass to None, it would just return data as is? :/
Actually, disregard that suggestion, the Serializer(data=data).create(klass) approach will break nested serializers.
@clarkduvall What do you think about https://github.com/clarkduvall/serpy/issues/3?
I'm now inclined to go back to the Meta inner-class approach that will allow us customize the serializer behavior cleanly.
Yeah I think as more features are added the Meta inner class will be better. Lets do that, we can also move default_getter and default_setter to the Meta class
I still want a make_object function instead of doing cls() and setting attributes on it. I think by default the deserialized output should be a dict, and users shouldn't need to specify any class so we don't assume too much about what the final deserialized output will be. If they want it in class form, they can use the make_object method.
@clarkduvall I'm confused, wouldn't make_object become no-op in default case then since it's input is already a dict? :/
yep, by default it would be a no-op unless you override it to return a class, see marshmallows implementation: https://github.com/marshmallow-code/marshmallow/blob/c4330c80073811bc175ffead89fe3cdc91797421/marshmallow/schema.py#L587-L595
For example to override it for a Django User model:
def make_object(self, data):
return User(**data)
then when you do serializer.deserialized_data it will return a User object
I see what you mean. What's your thought on keeping it no-op in the default implementation, but have another serializer that lets you pass in cls?
I just came across this great library. Deserialization would be a big win - I can't really contribute much at this point besides my gratitude.
I would be really thankful if deserialization compatible with DRF is implemented.
I'm using serpy in one of my projects and now I have to use DRF for deserialization and serpy for serialization. Having to define the same schema twice is rather unconvenient. Are there any plans when deserialization is going to be supported?
+1 for the make_object method rather than calling cls() as it allows for structures like {"type": "Foo", title="BlahBlah"} and {"type": "Bar", "title"="Blurb"} where the first is serialized as Foo and the second as Bar object. Keep it stupid simple and provide hooks to customize object creation. I also think that it is a good idea to pass the deserializer context to make_object. Otherwise it would be impossible to deserializer something like {"payload_type": "Foo", "payload_data": {"title": "BlahBlah"}}. That's a thing DRF does not support but Marshmallow does.
I don't have a lot of time to work on this now, but if someone wants to implement this (or @maroux wants to continue this PR) using the make_object interface that was discussed earlier, I would definitely take a look at it.