Add attrs.validators.or_ validator
#1303
Conversation
CodSpeed Performance ReportMerging #1303 will not alter performanceComparing Summary
|
Although good warning in general, in this particular code, they
do not fit here.
* BLE001 Do not catch blind exception: `Exception`
`validators` usually raise `ValueError` upon their violation.
However, it's not the only `Exception` they can raise - cf.
`attrs.validators.not_` - therefore, it is desirable to catch
them all!
* PERF203 `try`-`except` within a loop incurs performance overhead
Fair point, but the loop is written in a way to short-circuit,
ie. it will finish when a first validator is satisfied.
* S112 `try`-`except`-`continue` detected, consider logging the exception
Not applicable here, we care only if **all** validators raise
an exception, which is already accomodated for after the `for`
loop.
hynek
left a comment
There was a problem hiding this comment.
code's looking good; just a few smaller issues pls
docs/api.rst
Outdated
| >>> from typing import Union | ||
| >>> @define | ||
| ... class C: | ||
| ... val: Union[int, str] = field(validator=attrs.validators.or_(attrs.validators.instance_of(int), attrs.validators.instance_of(str))) |
There was a problem hiding this comment.
this is not a great validator because, and I may blow your mind here ;), is instance takes a tuple! So you could write this: attrs.validators.instance_of((int, str)) or even attrs.validators.instance_of(int | str) on modern Pythons.
Just make up something with numbers I guess?
There was a problem hiding this comment.
Yep, I was aware of the fact you can can isinstance with type argument being a tuple, but didn't put 2 and 2 together that it will be the case with attrs.validators.instance_of as well. 🤦🏻♂️
There was a problem hiding this comment.
Done in a0b6b24 .
I reworked the example to have one-or-many scenario, where the attribute can either a value of a type or a collection of values of the same type, encountered eg. in parsing the query parameters in HTTP URIs.
...?val=1&val=2&val=3Is this OK?
As an aside, docs seem to be tested wth py312, so I don't have to do the typing ceremony and can write val: int | list[int], but since there are python versions in the test matrix that don't have this syntax on by default, should I keep it as it is now or rework to conform to newer versions?
Thanks!
There was a problem hiding this comment.
Yes, please use the most-modern syntax. Let’s teach ppl modern features.
Example looks fine on a phone 😅
Co-authored-by: Hynek Schlawack <hs@ox.cx>
No problem, pleasure on my side for being able to contribute to a library I consider one of the foundational for the whole Python ecosystem! I thank you for timely responses and guidance. Cheers, |
Closes #1214
Summary
Add
attrs.validators.or_validator, which is satisfied if at least one of the validators it is wrapping is satisfied, thus complementing theand_andnot_"logical connectives" validators.Pull Request Check List
mainbranch – use a separate branch!Our CI fails if coverage is not 100%.
.pyi).tests/typing_example.py.attr/__init__.pyi, they've also been re-imported inattrs/__init__.pyi.docs/api.rstby hand.@attr.s()have to be added by hand too.versionadded,versionchanged, ordeprecateddirectives.The next version is the second number in the current release + 1.
The first number represents the current year.
So if the current version on PyPI is 22.2.0, the next version is gonna be 22.3.0.
If the next version is the first in the new year, it'll be 23.1.0.
.rstand.mdfiles is written using semantic newlines.changelog.d.TODO (done!)
changesdir, but that is dependent on the PR number of this PR, so needs to be post hoc - done in 6996627docs/api.rst- done in 2ad53b3