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

Remove Maybe validator typing #517

Merged
merged 2 commits into from
Jun 25, 2024

Conversation

cdce8p
Copy link
Contributor

@cdce8p cdce8p commented Jun 25, 2024

Ref home-assistant/core#120450 (comment)

The validator type for Maybe shouldn't be limited to Callable.
In the end it is passed to _WithSubValidators through validators which is currently untyped.

class _WithSubValidators(object):
"""Base class for validators that use sub-validators.
Special class to use as a parent class for validators using sub-validators.
This class provides the `__voluptuous_compile__` method so the
sub-validators are compiled by the parent `Schema`.
"""
def __init__(
self, *validators, msg=None, required=False, discriminant=None, **kwargs
) -> None:

Removed the incorrect typing from Maybe for now.

Refs home-assistant/core#120268

@alecthomas
Copy link
Owner

I don't really understand this change. All validators are callable, so this seems like it should be a valid constraint.

@alecthomas
Copy link
Owner

But maybe it should be using Schemable instead

@cdce8p
Copy link
Contributor Author

cdce8p commented Jun 25, 2024

All validators are callable, so this seems like it should be a valid constraint.

Validators can be callables but don't have to. Just as example, consider vol.Maybe({"key": int}).

Schemable seems to be a good alternative. Used that instead.

@alecthomas alecthomas merged commit 66df7be into alecthomas:master Jun 25, 2024
8 checks passed
@alecthomas
Copy link
Owner

Sweet, thanks.

@cdce8p cdce8p deleted the remove-maybe-typing branch June 25, 2024 23:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants