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

post_dump is passing a list of objects as original object #315

Closed
viniciuschiele opened this Issue Oct 29, 2015 · 8 comments

Comments

Projects
None yet
4 participants
@viniciuschiele

viniciuschiele commented Oct 29, 2015

Hi,

I think post_dump with pass_original=True should pass the original object related to the data serialized and not a list of objects which this object belongs to.

from marshmallow import fields, post_dump, Schema

class DeviceSchema(Schema):
    id = fields.String()

    @post_dump(pass_original=True)
    def __post_dump(self, data, obj):
        print(obj)  # <-- this is a list

devices = [dict(id=1), dict(id=2)]
DeviceSchema().dump(devices, many=True)

In the above example, the parameter obj is a list of devices rather than the device object itself.

What do you think?

@viniciuschiele viniciuschiele changed the title from post_dump pass a list of objects as original object to post_dump is passing a list of objects as original object Oct 29, 2015

@sloria

This comment has been minimized.

Member

sloria commented Nov 8, 2015

I'm not sure on this. I think it's intuitive for pass_original to pass the target object unmodified (i.e. if a collection is being serialized, the "original object" is the collection), but I understand if others might disagree. I am very open to feedback on this.

@viniciuschiele

This comment has been minimized.

viniciuschiele commented Nov 8, 2015

I understand your point of view, but for me it's more intuitive and useful to receive the Device instance instead of a collection, and I think it would be more consistent because I can serialize a single device or a collection of devices and my scheme would work for both cases, I also could use my schema in a Nested field and it would work too because I know it always gets a Device instance as original object. Thanks!

@frol

This comment has been minimized.

Contributor

frol commented Mar 6, 2016

+1 for @viniciuschiele's points. It is unexpected to receive a list instead of an object.

@deckar01

This comment has been minimized.

Member

deckar01 commented Jun 27, 2016

Since schema validators are designed to validate a single object instance unless pass_many is explicitly false, I think the intuitive behavior is for pass_original to return the corresponding original object instance unless pass_many is explicitly false.

  • Picking the original preprocessed object out of a list is inconvenient at best and impossible at worst.
  • Validation of a nested list is more appropriate on a parent schema instead of an overload of the child schema validator.
@deckar01

This comment has been minimized.

Member

deckar01 commented Nov 19, 2017

I was able to hack around this by using a temporary schema during serialization instead of many=True on the schema constructor.

I changed

data, _ = MySchema(many=True).dump(records)
return data

to

# HACK: Work around https://github.com/marshmallow-code/marshmallow/issues/315
class MyListSchema(Schema):
    items = fields.List(fields.Nested(MySchema))
data, _ = MyListSchema().dump({'items': records})
return data['items']

Luckily this hack is tucked away into a class that gets inherited by all my models, so I don't have to duplicate it everywhere. This hack allows me to avoid duplicating my schemas just to override post_dump to handle the original list.

@deckar01

This comment has been minimized.

Member

deckar01 commented Nov 21, 2017

This could be fixed without breaking backwards compatibility by allowing a special value like pass_original='single' to opt in the desired behavior. I would recommend making this the default behavior in the next major version though.

@sloria

This comment has been minimized.

Member

sloria commented Feb 25, 2018

OK, I think we can move forward with this. Let's go with the OP, summarized concisely by @deckar01 's comment above:

I think the intuitive behavior is for pass_original to return the corresponding original object instance unless pass_many is explicitly false true.

@stj has made a PR for this in #744.

EDIT: edit @deckar01 's comment to be correct.

@sloria

This comment has been minimized.

Member

sloria commented Mar 5, 2018

Re-opening this, since #744 only implements this behavior for validates_schema. This still needs to be implemented for post_load and post_dump.

@sloria sloria reopened this Mar 5, 2018

stj pushed a commit to stj/marshmallow that referenced this issue Mar 6, 2018

Stefan Tjarks
Fix pass_original on nested many
Pass the correct `original_data` for decorated `post_load` and
`post_dump` methods when dealing with a `Nested(many=True)` field.

Resolve marshmallow-code#315

stj added a commit to stj/marshmallow that referenced this issue Mar 6, 2018

Fix pass_original on nested many
Pass the correct `original_data` for decorated `post_load` and
`post_dump` methods when dealing with a `Nested(many=True)` field.

Resolve marshmallow-code#315

stj added a commit to stj/marshmallow that referenced this issue Mar 6, 2018

Fix pass_original on nested many
Pass the correct `original_data` for decorated `post_load` and
`post_dump` methods when dealing with a `Nested(many=True)` field.

Resolve marshmallow-code#315

stj pushed a commit to stj/marshmallow that referenced this issue Mar 6, 2018

Stefan Tjarks
Fix pass_original on nested many
Pass the correct `original_data` for decorated `post_load` and
`post_dump` methods when dealing with a `Nested(many=True)` field.

Resolve marshmallow-code#315

stj added a commit to stj/marshmallow that referenced this issue Mar 6, 2018

Fix pass_original on nested many
Pass the correct `original_data` for decorated `post_load` and
`post_dump` methods when dealing with a `Nested(many=True)` field.

Resolve marshmallow-code#315

@sloria sloria closed this in #750 Mar 7, 2018

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