Add a not_ validator

[nicktimko]

Summary

Still a bit of a WIP.

My original motivation for this validator was to enable constructions like not_(in_(["some", "reserved", "keywords"])), and rather than duplicating and swapping in for not in in the in_ validator, figured I could make it composable.

Pull Request Checklist

• Added tests for changed code.
  Our CI fails if coverage is not 100%.
• New features have been added to our Hypothesis testing strategy.
• Changes or additions to public APIs are reflected in our type stubs (files ending in .pyi).
  • ...and used in the stub test file tests/typing_example.py.
  • If they've been added to attr/__init__.pyi, they've also been re-imported in attrs/__init__.pyi.
• Updated documentation for changed code.
  • New functions/classes have to be added to docs/api.rst by hand.
  • Changes to the signature of @attr.s() have to be added by hand too.
  • Changed/added classes/methods/functions have appropriate versionadded, versionchanged, or deprecated directives.
    Find the appropriate next version in our __init__.py file.
• Documentation in .rst files is written using semantic newlines.
• Changes (and possible deprecations) have news fragments in changelog.d.
• Consider granting push permissions to the PR branch, so maintainers can fix minor issues themselves without pestering you.

[hynek]

Yeah sorry, this is a no-go. We can't just blanketly eat exceptions. Sorry for suggesting that! Feel free to submitting a not_in PR.

[nicktimko]

I guess I'm not sure how it "blanketly eats" the exceptions. I view validators as effectively returning a boolean: None or raise ValueError (raising being convenient as it can bubble up through a call stack without a bunch of extra handling). I suppose if you had an unexpected ValueError raised within the not-wrapped validator, it would be mysterious if it did successfully validate after getting inverted by the not... However, in that case it's the dual of testing some input should fail validation and it's not failing, so your test fails without an exception. You'll need to do some extra debugging in either case.

As I mentioned, not_in was good enough for my use case, but I've also used not re.match(...) and not isinstance(...) in other places (though not as attrs validators). I can prep a not_in PR.

[hynek]

I view validators as effectively returning a boolean: None or raise ValueError (raising being convenient as it can bubble up through a call stack without a bunch of extra handling).

This is not a safe assumption to make – validators can raise any exception and the fact that our internal ones usually raise ValueErrors (I think there’s a TypeError in there too tho?) is not a general rule.

Since attrs is not a validation library, but tries to help you write classes like you would, I encourage people to use any exception that makes sense to them.

---

That said, and after thinking some more about it, I could live with a not_, if it handles ValueError and TypeError and is super clear about that. I’ve just noticed you did the latter part and I have to admit, that I stopped reading after I saw the except ValueError. I’ll re-open and comment inline.

[hynek]

Thanks, sorry for the detour. It’s almost there, this should be quick to merge.

[nicktimko]

Set the default captured exceptions to (ValueError, TypeError) as suggested, and allowed whatever exceptions to be provided as the second argument to not_.

In order to validate them, I maybe went a bit overboard and modified the _InstanceOf to _SubclassOf. Could skip validation entirely I suppose like instance_of, which doesn't really care what's passed in, or rather, it doesn't do any 'compose time' validation, it just passes it off to isinstance(obj, ____) at 'check time'.

[hynek]

two things:

1. I know we forgot for a few validators, but all validators should also raise at least the attribute as the second argument. Ideally also what it expects and what it got instead but I'm not sure if that makes any sense here – you could make the validator an arg tho!
2. Should we maybe make the error message configurable? The current one can't be exposed to anyone using the class. E.g. not_(instance_of(str), msg="x must not be a str.")

[nicktimko]

I'll work on this over the weekend. I was just working from another validator's code, but I'll try to replicate a more recent one.

Was the subclass_of stuff abhorrent, or OK to leave in (just using it for 'internal-use' validation)?

[hynek]

I can live with the subclass stuff if it stays internal!

[nicktimko]

Should be good for re-review. I have two one lingering errors I don't know how to fix, first is with the typing, where mypy is giving me:

tests/typing_example.py:214: error: Argument "exc_types" to "not_" has incompatible type 
"Type[ValueError]"; expected "Union[Type[type], Iterable[type]]"

I'm speccing it in the .pyi as

def not_(
    validator: _ValidatorType[_T],
    *,
    msg: Optional[str] = None,
    exc_types: Union[Type[type], Iterable[type]] = (ValueError, TypeError)
) -> _ValidatorType[_T]: ...

which gets 'tested' as

    m: Any = attr.ib(
        validator=attr.validators.not_(
            attr.validators.in_("abc"), exc_types=ValueError
        )
    )

Though I think my problem is I don't know how to specify that the type is Exception types, rather than instances of exceptions

The other is Sphinx complaining that my type declaration of :type exc_types: Exception type or an iterable of Exception types. isn't a thing. Could just drop that I guess? (edit: I did)

[nicktimko]

Derp, fixed I think. Was doing Union[Type[Exception], Iterable[Exception]] but the latter in the union should have been Iterable[Type[Exception]] instead.

[nicktimko]

Will fix. Sorry to 🚲🏠, but another edit I'm noticing as I'm looking at what to put in the exception args, various validators seem to have different ideas:

• instance_of, matches_re, provides, in_
  • attr
  • a parameter of the validator
  • value that failed validation
• numbers (lt, le, gt, ge)
  • No additional args beyond a formatted message
• is_callable
  • Custom Error type
  • value as kwarg

Should replicate the top as you suggested and have the docs look like the others, e.g.:

... a human readable error message, the attribute (of type attrs.Attribute), the expected (interface|type|...), and the value it got.

The 'expected' part should probably be the wrapped validator that should have failed, but is it worthwhile tacking on the captured errors? As arg[4] at the end (note: where the PR is now), or before "the value it got." and keep that as arg[-1]?

[nicktimko]

Anything else to do? The only checkmark I'm not sure about is the hypothesis testing: none of the other validators have it, and when I started on it, it felt very much like I was reimplementing the same logic to derive what should be expected from various permutations received.

[hynek]

Alright thanks!

One minor nit that I'll fix myself: don't put default values into type stubs but a ....