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

Ensure Maybe propagates error information #411

Merged
merged 1 commit into from
Dec 6, 2020

Conversation

monopolis
Copy link
Contributor

Due to Any evaluating all options, and raising the exception
from the first encountered error, Maybe would discard the
expected error raised by the provided validator.

This commit changes the order of the validators in Maybe so
that the first evaluated error (and thus returned) is that of
the provided validator.

@coveralls
Copy link

coveralls commented Nov 7, 2019

Coverage Status

Coverage increased (+0.8%) to 95.617% when pulling b4e3e8d on monopolis:issue-369 into 7151438 on alecthomas:master.

@svisser
Copy link
Collaborator

svisser commented Nov 19, 2019

Hmm, it's interesting because the test case now fails in Python 3.x due to:

  File "/home/travis/build/alecthomas/voluptuous/voluptuous/validators.py", line 589, in __call__
    if self.min is not None and not v >= self.min:
TypeError: '>=' not supported between instances of 'NoneType' and 'int'

This previously didn't happen as it checked for None first.

@monopolis
Copy link
Contributor Author

Please see pull request #414 for a potential fix for the incomparable values. It should solve this issue at the very least.

My guess is that there probably are other validators that should check for TypeError's as well - e.g. Length comes to mind.

@spacegaier
Copy link
Collaborator

Please see pull request #414 for a potential fix for the incomparable values. It should solve this issue at the very least.

My guess is that there probably are other validators that should check for TypeError's as well - e.g. Length comes to mind.

@svisser The mentioned PR #414 has by now been merged and indeed fixes this issue for me.
@monopolis I added additional TypeErrors to Clamp and Length, so that is covered now.

Due to Any evaluating all options, and raising the exception
from the first encountered error, Maybe would discard the
expected error raised by the provided validator and only
raise a invalid value error (due to values not being None).

This commit changes the order of the validators in Maybe so
that the first evaluated error, and thus returned is that of
the provided validator.
@spacegaier spacegaier changed the title Maybe propagates error information (#369) Ensure Maybe propagates error information Dec 6, 2020
@spacegaier spacegaier linked an issue Dec 6, 2020 that may be closed by this pull request
@spacegaier spacegaier merged commit da3cc96 into alecthomas:master Dec 6, 2020
Tankanow added a commit to Tankanow/voluptuous that referenced this pull request Dec 7, 2020
alecthomas pushed a commit that referenced this pull request Feb 19, 2021
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.

Maybe loses error information
4 participants