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

Inconsistent behaviour of load_instance with nested schemas #400

Open
zedrdave opened this issue Jul 19, 2021 · 4 comments
Open

Inconsistent behaviour of load_instance with nested schemas #400

zedrdave opened this issue Jul 19, 2021 · 4 comments

Comments

@zedrdave
Copy link

Thanks to this MR, it is now possible to define load_instance at the instance level (MySchema(load_instance=False)).

However, as far as I can tell, this setting is not inherited by nested schemas, and there is no easy way to make the nested schema behave like its parent… Resulting in half-loaded schemas (top-level is a dict, with instances underneath).

Could we consider changing that, or is there an obvious workaround that I am missing?

@AbdealiLoKo
Copy link
Collaborator

AbdealiLoKo commented Jul 20, 2021

I'm not sure what inherited by nested schemas exactly means

If I understand this right - you should just be able to do:

prop = fields.Nested(MySchema(load_instance=False))

Similar to: https://marshmallow.readthedocs.io/en/stable/nesting.html#specifying-which-fields-to-nest

@zedrdave
Copy link
Author

@AbdealiJK you are able to define a nested schema as explicitly load_instance=False.

But I am talking about the use-case where you have a schema with a nested schema:

class Album(ma.Schema):
    name = ma.fields.String()
    artist = ma.fields.Nested(ArtistSchema)

In this case, if you use Album(load_instance=False) to get a non-loading version of the schema, the nested artist field will still be loading. If you add the load_instance=False into the schema declaration, then you lose the whole flexibility of choosing which version you want at instantiation time.

Right now, I am working around this, by manually setting nested fields:

            UpdateSchemaInst = SchemaCls(partial=True, load_instance=False)
            for key, field in UpdateSchemaInst.declared_fields.items():
                if isinstance(field, ma.fields.Nested) and not field.dump_only:
                    field.nested = field.nested(load_instance=False)

but it seems like this should be handled by the mix-in, no?

@AbdealiLoKo
Copy link
Collaborator

I see, got it - I recall some similar discussions regarding the partial kwarg too
See: marshmallow-code/marshmallow#438

I stopped following the thread midway - and don't know the current implementation approach it took
But just referencing it here for anyone looking to work on this

@zedrdave
Copy link
Author

Thanks, that makes good sense: we very likely would want to support nested load_instance exactly the same way as partial

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

No branches or pull requests

2 participants