deep_iterable and deep_mapping inconsistencies

[AdrianSosic]

Hi 👋 In the last days, I noticed some inconsistent behavior of deep_iterable and deep_mapping. Some of the problems have already been mentioned in other issues (links provided below) but for others it seems there is no open issue yet. Since they are partly related, I thought it would be good to have a single overview of the necessary open fixes:

Lists/tuples of validators

Some of the inner validators accept lists while others still require the explicit use of and_. For consistency, I think all of them should ideally be able to handle lists. For the member_validator, the change has been in implemented in #925, but iterable_validator still struggles with lists. For example, the following piece of code works, but replacing any_ with a list will make it fail:

@define
class A:
    x: List[str] = field(
        validator=deep_iterable(
            member_validator=[instance_of(str), min_len(1)],
            iterable_validator=and_(instance_of(list), min_len(1)),
        ),
    )

For deep_mapping, lists/tuples not supported for any of its three arguments.

Typing support

When a list of validators is passed, mypy complains about the type, as already pointed out in #1197. However, when a tuple is used instead, mypy is happy.

Wrong error message

When a validation error in one of the inner validators of deep_iterable or deep_mapping occurs, an exception is thrown but the contained message is wrong. For example, using the above code and calling A(["abc", ""]), you get the following error:

ValueError: Length of 'x' must be => 1: 0

It rightfully complains about the length of the second item in the list, but note that the message refers to attribute name x, which is incorrect, since it's not x that is too short but one of its items.

Optional validators

For deep_iterable, only the iterable_validator is optional, which makes sense because if no member_validator was provided, one should simply use a regular validator in the first place. However, for deep_mapping, both value_validator and key_validator are required. This makes its use unnecessarily complicated in certain situations. For example, validating only dictionary keys requires a rather cumbersome workaround and also results in unnecessary calls of the value_validator:

@define
class B:
    x: Dict[str, Any] = field(
        validator=deep_mapping(
            key_validator=and_(instance_of(str), min_len(1)),
            value_validator=lambda *x: None,
        )
    )

Instead, I guess the user-friendly way would be to check that at least one of them is provided, but not necessarily both.

[hynek]

Thanks for the summary – would you mind filing the bugs (additionally) individually, so we can check them off? Have you perchance debugged why the error message is wrong?

[AdrianSosic]

Sure, will do so 👍🏼

Regarding your question about the error message: Well, I think there is just not enough context information passed when calling the validators on the individual members. I guess what we would ideally like to have is some message that provides the name of the iterable plus the index of the failing element, e.g. in this case

ValueError: Length of 'x[1]' must be => 1: 0

Of cource, even better would be an ExceptionGroup that reports all failing elements, like cattrs would do it.

However, these are the current two lines from _DeepIterable that trigger the validation and there is simply no information about the index passed.

for member in value:
    self.member_validator(inst, attr, member)

[AdrianSosic]

Hi @hynek, here you go, the four issues are now separately tracked in #1243, #1244, #1245 and #1246 😎. So feel free to close this issue if you prefer to keep only the individual ones 👍🏼

[hynek]

yes thanks we've got enough open ones 🙈