marshmallow-sqlalchemy icon indicating copy to clipboard operation
marshmallow-sqlalchemy copied to clipboard

Inconsistent behaviour of load_instance with nested schemas

Open zedrdave opened this issue 4 years ago • 5 comments

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?

zedrdave avatar Jul 19 '21 15:07 zedrdave

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

AbdealiLoKo avatar Jul 20 '21 05:07 AbdealiLoKo

@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?

zedrdave avatar Jul 20 '21 05:07 zedrdave

I see, got it - I recall some similar discussions regarding the partial kwarg too See: https://github.com/marshmallow-code/marshmallow/issues/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

AbdealiLoKo avatar Jul 20 '21 05:07 AbdealiLoKo

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

zedrdave avatar Jul 20 '21 05:07 zedrdave