Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support validator=, validators=, or validate_with=. #645

Closed
douglas-treadwell opened this issue Jun 10, 2017 · 4 comments
Closed

Support validator=, validators=, or validate_with=. #645

douglas-treadwell opened this issue Jun 10, 2017 · 4 comments

Comments

@douglas-treadwell
Copy link

In field constructors, validate=validation_function doesn't flow naturally in English.
An average person, before reading the docs, might think that a parameter named "validate" would take a boolean and turn validation on or off, like "strict".

For a single validator, Field(validator=is_lowercase) sounds more natural.
For multiple validators, Field(validators=[is_lowercase, is_alphanumeric]) sounds more natural.
For convenience, these could be aliases of the same method, although I can't think of a case where a developer-user of this API would not know whether they're validating with one or multiple functions.

@monotaro-yasushi-masuda

+1 for validator. Even if validator accept multiple validators, it can be recognized as "a validator bulit with a combination of validators".
-1 for validate_with, parameter key better not to have particles, except the case such as valid_from and valid_to etc.

@lafrech
Copy link
Member

lafrech commented Apr 15, 2018

I agree validator would be more natural than validate, and validate_with is too long.

Currently, validate accepts both a callable and a collection of callables and I'd rather stick to only one version, either validator or validators.

validators=my_callable

definitely looks weird.

validator=(collection of callables)

could do, like @monotaro-yasushi-masuda says, it is a compound validator so singular form is understandable, but it is still surprising to provide a collection to a singular argument.

The most natural thing, of course, would be to use validators and accept only collections. Besides, internally the value is stored as self.validators, so it is clearer to expose this to the caller, so that the parameter matches the attribute like for the other attributes.

But most of the time we only use one validator, so the shortcut allowing a callable is nice.

There's a trade-off here between clarity and practicality (and history).

Keeping the ability to pass only a callable would be nice. In this case, validate comes in handy precisely because it does not indicate whether it is plural or singular.

At least, keeping validators inside does not break introspection for third-party libs (like apispec).

@sloria
Copy link
Member

sloria commented Oct 19, 2018

validate comes in handy precisely because it does not indicate whether it is plural or singular.

IIRC, this was the reason for choosing "validate." I wouldn't consider validator=[list of callables] any more natural than validate=[...]. validator=callable is slightly more natural than validate=callable, but I'm not sure it's worth the breaking change.

@sloria sloria added this to the 4.0 milestone Jul 14, 2019
@sloria
Copy link
Member

sloria commented May 19, 2020

Closing this, as it has gone stale and I'm still not convinced that the OP is a significant improvement over the status quo to justify a breaking change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants