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

thb/fix-283 #298

Merged
merged 2 commits into from
Nov 2, 2017
Merged

thb/fix-283 #298

merged 2 commits into from
Nov 2, 2017

Conversation

tbillon
Copy link
Contributor

@tbillon tbillon commented Jul 12, 2017

This PR revert PR #215 to address #283. To make things more consistent, make an empty list raise an exception like an empty dict.

So an empty dict gets its path variable back:

>>> from voluptuous import Schema
>>> Schema({})({"extra": "invalid", "extra2": "invalid", })
Traceback (most recent call last):
...
voluptuous.error.MultipleInvalid: extra keys not allowed @ data['extra2']

And now an empty list also raise a MultipleInvalid exception and set the path variable:

>>> from voluptuous import Schema
>>> Schema([])(['list_invalid', 'list_invalid2'])
Traceback (most recent call last):
...
voluptuous.error.MultipleInvalid: not a valid value @ data['list_invalid']

Validation on an empty list was not raising any exception when given values
but an empty dict was. Make it uniform and make them both raise a
MultipleInvalid exception on unwanted values.
@coveralls
Copy link

coveralls commented Jul 12, 2017

Coverage Status

Coverage increased (+0.009%) to 95.319% when pulling 6af92d1 on tbillon:thb/fix-283 into 77ee5da on alecthomas:master.

@brmzkw
Copy link

brmzkw commented Jul 24, 2017

Any plan to merge this?

Copy link
Collaborator

@tusharmakkar08 tusharmakkar08 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure about this since it is contrast with the issue mentioned here. @alecthomas Please look into this.

@brmzkw
Copy link

brmzkw commented Oct 27, 2017

hi :)

@tusharmakkar08
Copy link
Collaborator

Hey @brmzkw

I can't take a call on this alone. Would require @alecthomas approval for the same.

Thanks.

@alecthomas alecthomas merged commit 95489bd into alecthomas:master Nov 2, 2017
@alecthomas
Copy link
Owner

LGTM, thanks.

@brmzkw
Copy link

brmzkw commented Nov 2, 2017

since backward compat was broken, do you think you can make a release soon?

@tuukkamustonen
Copy link
Contributor

@tbillon Care to clarify this change a bit:

  1. So the change here was to revert {} behavior - if extra=True it allows extra params (inherited) and if not then it doesn't? Correct me if wrong.
  2. Why remove whole test_empty_dict_as_exact() and not modify it (most parts still apply)?
  3. This change is marked as fix in CHANGELOG.md - should it be marked as change instead?

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.

None yet

6 participants