Bug: several nested partial issues (sequence partial, data key, schema initialized partial)

[pavellos21]

Hello there! I was playing with partial attribute for Nested fields and faced several issues related to them.

1. When you providing sequence as partial instead of bool nested schema continue to be required:

   class SchemaA(Schema):
       first_nested = fields.Integer(required=True)
       second_nested = fields.Integer(required=True)
   
   
   class SchemaB(Schema):
       original = fields.Nested(SchemaA)
   
   
   test_data = {"original": {"second_nested": 42}}
   
   result = SchemaB().load(test_data, partial={"original"})

   It'll raise ValidationError: {'original': {'first_nested': ['Missing data for required field.']}} but expected validation without errors.

2. When you providing partial argument while initializing Nested field it'll have no effect:

   class SchemaA(Schema):
       # the same as previous
       first_nested = fields.Integer(required=True)
       second_nested = fields.Integer(required=True)
   
   
   class SchemaB(Schema):
       original = fields.Nested(SchemaA(partial=True))
   
   
   test_data = {"original": {"second_nested": 42}}
   
   result = SchemaB().load(test_data)

   It'll raise the same ValidationError: {'original': {'first_nested': ['Missing data for required field.']}} error instead of passing validation.

While digging into the reason of mentioned issues I found the code which potentially causing another error. Testing showed that I'm right. So:

3. When schema fields have specified data_key providing sequence as partial must contain data keys instead of original schema defined field names:

   class SchemaA(Schema):
       first_nested = fields.Integer(required=True, data_key="frst_nst")
       second_nested = fields.Integer(required=True, data_key="snd_nst")
   
   
   class SchemaB(Schema):
       original = fields.Nested(SchemaA, data_key="orig")
     
     
   test_data = {"orig": {"snd_nst": 42}}
     
   result = SchemaB().load(test_data, partial={"original.first_nested"})

   It'll raise ValidationError: {'orig': {'frst_nst': ['Missing data for required field.']}}. To make it work you forced to use "orig.first_nested" instead. I believe that this approach isn't correct and we need to use field name, not data key.

All of the mentioned issues fixed in the PR that you can see in the attachments below.

[lafrech]

Hi. Thanks for the report and PR.

1. I think current behaviour is correct.

This line

result = SchemaB().load(test_data, partial={"original"})

says that original may be omitted. However, you're not omitting it, you're providing an incomplete "original".

test_data = {"original": {"second_nested": 42}}

What you want to do is

result = SchemaB().load(test_data, partial={"original.first_nested"})

2. I think you're right, although I'm wondering if people could be relying on current behaviour.

3. Yes, this is definitely a bug.

[pavellos21]

Thank you for respond!

Regarding first point... Do the library have any other way to mark nested field as fully partial then? By "fully partial" I mean the field by itself with all children of this field recursively. Might it be "original.*" or something?

In current PR I can remove code which touching logic for first point to merge fixes for 2nd and 3rd issues. Then as soon as we'll know what to do with the first one I can open another PR. What do you think?

[pavellos21]

One more thing regarding first point... If it's expected that only selected field marked as partial does it mean that passing True into partial should be non-recursive? Currently if we pass True all nested fields will be marked as partial as well. Is it correct behavior?

[CtrlC-Root]

I just ran into issues 2 and 3 from the bug report above. Currently working around it with a custom decorator based on #438 (comment).

def _partial_schema(schema_cls, **kwargs):
    """
    Create a modified instance of a schema class that has all of it's fields
    configured to support partial loading and dumping.

    https://github.com/marshmallow-code/marshmallow/issues/438#issuecomment-673428463
    """

    if 'partial' not in kwargs:
        kwargs['partial'] = True

    schema = schema_cls(**kwargs)
    for field in schema.fields.values():
        # do not require field value
        field.required = False

        # do not provide default value when loading or dumping
        field.load_default = missing
        field.dump_default = missing

    return schema

[npalmius]

For what it's worth, I agree that point 1 is by design, but it might be nice to have a separate deep_partial (or similar) option to propagate into nested fields.

[matuqam]

2. I think you're right, although I'm wondering if people could be relying on current behavior.

Apologies if this hypothesis has been confirmed already but, if indeed it does not have any effect to do partial=True on a fields.Nested then one could think that no one does it as it would be futile to do so and therefore changing this wouldn't break anyones code?

[lafrech]

Possibly. People could also be using a partial instance created earlier and used in a place where partial is needed. But I admit this is a bit convoluted. Anyway, fixing this could arguably be considered a bugfix.

[hnandiwada]

+1 for a passed-through partial or deep_partial field. This is causing a little bit of degradation in our product and a workaround to propagate it would be a huge lift. We designed our code around arguments like this being propagated.