W dniu 11.07.2012 14:04, Russell Keith-Magee pisze:
There is still problem with API and how to do some things but in my opinion
it's going in right direction.
Generally, I agree. I still have some concerns however; mostly around
the things that you're putting onto the Meta class.

related_serializer, for example -- Why is this a single attribute in
the meta, rather than a method? By using an attribute, you're saying
that on any given serializer, *all* related objects will be serialised
the same, and I don't see why that should be the case.
Not *all* related objects but only those that aren't declared in class definition. I think related_serializer attribute is useful when you want to serialize all related object in one way: to their's primary key value, to their's natural key value, to dumpdata format. If you want to do exception for some fields then you declare it in class definition.


class MySerializer(ModelSerializer):
    special_object =  SpecialSerializer()
    class Meta:
        related_serializer = PkSerializer

In this case all related objects except special_object will be serialized to pk value.

What you will do more with a related_serializer method? If you want to serialize some related objects by one serializer and some by another the simplest way to do it is declare this in class definition. I see only two examples when method will be needed. If you want to get serializer by some pattern in field name or if you want to get serializer by related object type (m2m, fk). Then you can override get_object_field_serializer(self, obj, field_name) method to do it. Default this method return related_serializer or field_serializer based on field type. Maybe good idea will be to split this method to two, one for related object and one for non related. Then overriding it will be very similar to set attribute in Meta, but I think attributes are more "declarative".

The same argument goes for class_name (which I think I've mentioned
before), field_serializer, and so on.
And there is method for that :)

def create_instance(self, serialized_obj):
    if self.opts.class_name is not None:
        if isinstance(self.opts.class_name, str):
            return _get_model(serialized_obj[self.opts.class_name])()
        else:
            return self.opts.class_name()
raise base.DeserializationError(u"Can't resolve class for object creation")

Maybe it isn't proper way to do this - there is two ways to doing same operation, but I think this is simplest solution for end user.

The only fields that I can see
that *should* be declarative are 'fields' and 'exclude' -- and if
you've been tracking django-dev recently, there's been a discussion
about whether the idea of 'exclude' should be deprecated from Django
APIs (due to potential security issues -- explicit inclusion is safer
than implicit inclusion, because you can accidentally forget to
exclude sensitive data from an output list)
I have read this discussion. I'm +1 to deprecate 'exclude' :) Personally I almost never use it.


Some other API questions:

Why is deserialized_value decoupled from set_object? It isn't obvious
to me why this separation exists.
It's possible that I overcomplicated this. There is three methods: set_object, deserialize and deserialize_value. When you want to deserialize object then you should: * Ensure that this is proper object not list of objects or dict (dict in deserialization is another problem - I will present it below) - 'deserialization' method will handle this - it recursively deserialize lists and dicts. * Do some processing on object you get ( e.g. change string to int) 'deserialize_value' method will handle this * Set this object to upper level object. 'set_object' method will handle this. There shouldn't be reason to override it very often.

I think deserialize_value will be method that user would most often needed to override. I would be acquiescent to merge deserialize and deserialize_value. But set_object should be left as is.

Problem with deserializing dict:
In current implementation in deserialization there is no way to guess that given dict is serialized object or it is dict of objects. So it might be better to don't automatically serialize dicts but leave it to the user decision?

I see where you're going with metainfo on fields (and that's a
reasonably elegant way of tackling the problem of XML needing
additional info to serialize), but what is the purpose of metadata on
Serializers?

Yours, Russ Magee %-)

Because Serializer should also have possibility to give additional info to format serializer. For example which fields should be treat as attributes (pk and model in dumpdata).


--
Piotr Grabowski

--
You received this message because you are subscribed to the Google Groups "Django 
developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Reply via email to