lafrech
commented
6 years ago [Let's strip this from OP, as it is long enough already.]
BTW, note that there can be errors for a missing field if a schema-level validator (with skip_on_field_errors=False) raises errors and blames them on the nested field, for instance:
raise ValidationError('Some error', 'nested')so this is possible:
{'nested': ['Missing data for required field.', 'Other error from schema level validator.']}Worse, if nested fields could be reached using dot notation (I don't think it is possible right now), you could even have errors for fields deeply nested in a missing nested field:
raise ValidationError('Some error', 'nested.nested_again.field')
errors = {
'nested': {
'_field': ['Missing data for required field.'],
'nested_again': {
'field': ['Some error']
}
} This sounds wrong so it's probably better not to support this.
sloria
Generally, a field name is associated a list of errors as strings. The exception to this is container fields. Those may return errors for the container and the contained field(s). In this case, rather than a list, a
dictis used where the keys are the field names or the list index (as string or as integer):This has been working relatively fine for a while. It leaves a hole as you can't return errors relative to the
Nestedfield itself, for instance, while returning errors for the fields in the nested schema.It is generally not blocking, as the most frequent
Nestedfield level error is themissingerror, and when the field is missing, there is generally no nested error, so we returnIt is a shame that the structure (
dictorlist) depends on the error, but we could live with this.Except we sometimes need to return errors for both the field and the nested schema, so we came up with this (https://github.com/marshmallow-code/marshmallow/pull/299):
where
_fieldstores errors about the field itself.This is inconsistent, as I said in https://github.com/marshmallow-code/marshmallow/pull/299#issuecomment-401607834 and showed in https://github.com/marshmallow-code/marshmallow/pull/863 because the errors on the nested field itself may be reported in different places depending on whether there are errors in the nested schema. In other words, a
dictwith a_fieldattribute is used only if there are also errors in the nested schema, otherwise, the errors are returned as a strings, like for any field.The same considerations apply to all container fields (
List,Dict).I can't think of an ideal solution to allow reporting errors from both the container field and its nested schema/fields.
We'd like to keep the use of
dictfor containers because it makes browsing the error structure convenient:This rules out putting nested errors under a dedicated
_nestedkey.On the other hand, putting errors on fields themselves in a dedicated
_fieldkey for all fields seems a bit overkill as well.A trade-off would be to use the
_fieldkey for fields errors for all container fields and only for them, but do it consistently.I suppose that was the intent of https://github.com/marshmallow-code/marshmallow/pull/299.
This introduces a distinction between containers and other fields.
It looks bad in the usual case where fields are missing, as shown above, because containers have their "missing data" message embedded in a
dict.Also, the implementation might be challenging: how do you know a field is a container when registering the error?
I'd like to keep it simple, but I'm out of ideas, so I'm appealing to collective wisdom.
I have the feeling that a good decision on this would avoid many corner case issues, so this should be set before piling-up patches.