Skip to content
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

fields.Nested should optionally pass the parent object to the Nested Schema #940

Open
remeika opened this issue Sep 7, 2018 · 12 comments
Open

Comments

@remeika
Copy link
Contributor

remeika commented Sep 7, 2018

As described in #900, there is no obvious way to create a nested serialization from a flat object. To fix this, fields.Nested should accept an optional from_parent argument. When true, the nested Schema is passed the parent object rather than the attribute at the field name. from_parent will default to False, which will maintain current behavior.

Any feedback on this idea? I plan on implementing it soon.

Thanks,
James

@sloria
Copy link
Member

sloria commented Oct 18, 2018

I'm not opposed to the idea. Feel free to send a PR for review. However, I can't promise we'll get to it in the immediate future because we're currently focused on the 3.0 final release.

@timc13
Copy link

timc13 commented Oct 19, 2018

I don't think this should be a concern of the serialization layer really.

Maybe a fix for the problem described in #900 is to add a computed property:

@property
def meta(self):
    return {'created': self.created, 'updated': self.updated}

@sloria
Copy link
Member

sloria commented Oct 21, 2018

@timc13 That's a good point. @remeika Is your use case solved by using a computed property like in Tim's example?

@lafrech
Copy link
Member

lafrech commented Oct 21, 2018

Related SO question: https://stackoverflow.com/questions/50485096/. I proposed @timc13's solution in an answer.

Shortcomings:

  • Boilerplate: need to create a mapping in the property function, while Marshmallow takes care of that in normal cases (including things like attribute and data_key)

  • I was gonna say it is one-way only, but I suppose you could also make an equivalent setter, it will just be even more boilerplate

@remeika
Copy link
Contributor Author

remeika commented Oct 25, 2018

@timc13 That is a totally valid workaround and addresses my case. But I still think this feature should be added, and here is why:

  1. Minor reshaping of one's data is a valid concern of the serialization layer. Marshmallow already has several affordances to perform more complex transformations of the source object: Method/Function fields come to mind, which already allow access to the parent object within a field.
  2. This feature will allow many more cases to use a purely declarative serializer. This is quite subjective, but I think less imperative code in a Serializer implementation is always a primary goal.

@lafrech
Copy link
Member

lafrech commented Nov 16, 2018

Could this be addressed by adding a pre_dump method that would copy the parent into the nested field?

class MySchema(Schema):

    nested = fields.Nested()
    ...

    @pre_dump
    def prepare_nested(item)
        item['nested'] = item
        return item

This way, the field does not have to access parent object. See #1046.

@dsully
Copy link

dsully commented Dec 15, 2018

+1 - I could really use this feature. The legacy structure I'm adapting can have nested fields at multiple levels, and it'd be really great to understand which one a field belongs to.

@remeika
Copy link
Contributor Author

remeika commented Dec 19, 2018

@lafrech Isolating fields from each other is a great idea, but is an implementation detail.

@m-a-choquette
Copy link

Hi,
It feels a little weird to set a property on the object while serializing it.

To work around this, we did something like this:

def identity_accessor(attr, obj, default):
    return obj or default


class Section(fields.Nested):
    def get_value(self, attr, obj, accessor=None, default=missing):
        return super(Section, self).get_value(attr, obj,
                                              accessor=identity_accessor,
                                              default=default)

Each Section field receives the full object.

@dlavelle7
Copy link

Best way to solve this problem is to set the "attribute" kwarg on the Nested field itself to a callable that returns the root object:

bar_schema = api.model("Bar", {
    "id": fields.String(),
    attribute="bar_id"
})

foo_schema = api.model("Foo", {
    "bar": fields.Nested(
        bar_schema,
        attribute=lambda root_obj: root_obj)})

{"bar_id": "123"} => {"bar": {"id": "123"}}

@multimeric
Copy link

I also solved this using a (simpler) variant of @m-a-choquette's method:

class SelfNested(fields.Nested):
    def get_value(self, obj, attr, accessor=None, default=missing):
        return obj

This let me use this schema:

class SampleFilterSchema(with_metaclass(SchemaMeta, BaseModelSchema)):
    class Meta:
        model = SampleFilter

    id = fields.Integer(attribute='sample_filter_id')
    type = fields.Constant('filter')
    attributes = SelfNested(Schema.from_dict(dict(
        tag=fields.String(attribute='sample_filter_tag'),
        name=fields.String(attribute='sample_filter_name'),
        public=fields.Boolean(attribute='is_public'),
        data=JsonString(attribute='sample_filter_data'),
        user=ResourceHyperlink(endpoint='rest_api.user', url_args=[
            'user_id',
        ])
    )))

(ignore my custom fields, they're not important for this example)

Which transforms:

{  
   "user_id":1,
   "is_public":false,
   "sample_filter_name":"TestFilter",
   "sample_filter_data":"",
   "sample_filter_tag":"Global",
   "sample_filter_id":1
}

into

{
   "id":1,
   "attributes":{
      "user":"/rest_api/v1/users/1",
      "tag":"Global",
      "public":false,
      "name":"TestFilter",
      "data": ""
   }
}

aka, a valid response (or segment thereof) under the JSON API standard!

@sloria
Copy link
Member

sloria commented Aug 29, 2019

There's discussion on #1315 (comment) to allow data_key to be a nested path specified as a list of strings which would address the issue here, I think. Please comment there (or even 👍 👎 ) if you have thoughts.

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

No branches or pull requests

8 participants