Skip to content

Pass correct initial_data to ListSerializer child (#5345) #5537

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

daggaz
Copy link
Contributor

@daggaz daggaz commented Oct 27, 2017

Note: Before submitting this pull request, please review our contributing guidelines.

Description

refs #5345

Thought I'd have a bash at this too.

I left setting initial_data when we iterate over child.to_representation(...). In theory you might want access to it there too, because you could be doing an "update":

Serializer(data=[...], instance=[...]).data

But I guess there's nothing to stop those list being different lengths or in different orders so mapping from one to the other isn't really sensible. I figured this is fine since ListSerializer doesn't support update() out of the box anyway.

Copy link
Collaborator

@carltongibson carltongibson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm inclined to say no to this. I think it adds a chunk of noise for a marginal use-case.

I'd prefer to see this added to a project-level ListSerializer subclass rather than added to DRF itself.

@encode/django-rest-framework-core-team Anyone got a view on this?

@carltongibson
Copy link
Collaborator

@daggaz Whilst I do see the point of it, I'm going to say no to this as an addition to the core framework. For me the benefit doesn't seem worth the extra complexity.

If you were to were to wrap this up as a third party package we could happily link to this from the docs. Users could then customise their list serialiser class. I think that's the best option.

Thanks for the input!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants