marshmallow-code/marshmallow

Allow 'load_default' for required fields since they can be used with partial.

matejsp opened this issue · 3 comments

Currently we are using load_default and required together for some cases where we are working with partial loading.

In this case field that is required is no longer required and should use load_default value, but it is not allowed in the code:

        if required is True and load_default is not missing_:
            raise ValueError("'load_default' must not be set for required fields.")

Would it make sense to allow it for such cases?

    first_party_non_custodial_allowed = marshmallow.fields.Bool(
        required=True,
        load_default=True,
        dump_default=True,
    )

I guess you're right. Removing the check would allow users to specify a default value for required fields on partial loading, while still raising an error on non-partial loads. I don't see any obvious downside to this.

Would you like to send a PR that removes the check and adds tests?

I just realized that load_default is not used when using partial anyway.

This is not explicit in the doc and in fact I'm wondering if this was intended in the first place but it's probably hard to change now as people may be relying on it.

See marshmallow-code/flask-smorest#538 (comment).

Is this something that should be reconsidered in marshmallow 4? Or opted-out in marshmallow 3.x?

Yeah no need to change it in 3.x since. And based on my testing it worked the same in 2.x (ignoring load_default).

For 4.x this could be maybe interesting. But don't know what is developer expectation regarding this. You could still exclude + partial (to have possibility to use load_default or not).

Maybe there is question what happens in a case:

from marshmallow import Schema
from marshmallow import fields


class UserSchema(Schema):
    name = fields.String(required=True)
    age = fields.Integer(load_default=123)
    age2 = fields.Integer(load_default=123)


result = UserSchema().load({"age": 42}, partial=("name", "age", "age2"))
print(result)

{'age': 42}

Should age2 get load_default or not? Or you should add validation that you are using partial not allowed for not required field :D