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.

Sign up for GitHub

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

WIP: Support namedtuple validation #365

Closed
wants to merge 1 commit into from
Closed

WIP: Support namedtuple validation #365

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,10 @@ _static
_templates

TODO

_trial_temp/
Copy link
Author

Choose a reason for hiding this comment

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

This is all stuff created when running tox locally.

Copy link
Member

Choose a reason for hiding this comment

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

The reason these aren't here already is because generally I'd recommend these go in your global git ignore, since they're things that should be ignored from all repos. E.g., mine is: https://github.com/Julian/dotfiles/blob/master/.config/git/ignore

But tbh I'm fine to merge it anyhow since I have had to explain ^ quite a few times so clearly at least some people are tripped up by it.

.eggs/
.tox/
build/
*.pyc
*.egg-info
35 changes: 34 additions & 1 deletion jsonschema/tests/test_validators.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from collections import deque
from collections import deque, namedtuple
from contextlib import contextmanager
from unittest import TestCase
import json
Expand Down Expand Up @@ -960,6 +960,39 @@ class TestDraft3UniqueTupleItems(UniqueTupleItemsMixin, TestCase):
validator_class = Draft3Validator


ParentTuple = namedtuple('ParentTuple', ['some_int', 'some_child'])
ChildTuple = namedtuple('ChildTuple', ['some_string'])


class TestNamedTuples(TestCase):
def setUp(self):
self.schema = {
'type': 'object',
'properties': {
'some_int': {
'type': 'integer'
},
'some_child': {
'type': 'object',
'properties': {
'some_string': {
'type': 'string'
}
}
}
}
}
self.validator = Draft4Validator(self.schema)

def test_standard_case(self):
instance = {'some_int': 1, 'some_child': {'some_string': 'hi'}}
self.assertEqual(list(self.validator.iter_errors(instance)), [])

def test_tuple_case(self):
instance = ParentTuple(some_int=1, some_child=ChildTuple(some_string='hi'))
self.assertEqual(list(self.validator.iter_errors(instance)), [])


def sorted_errors(errors):
def key(error):
return (
Expand Down
12 changes: 10 additions & 2 deletions jsonschema/validators.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ def create(meta_schema, validators=(), version=None, default_types=None):
if default_types is None:
default_types = {
u"array": list, u"boolean": bool, u"integer": int_types,
u"null": type(None), u"number": numbers.Number, u"object": dict,
u"null": type(None), u"number": numbers.Number,
u"string": str_types,
}

Expand Down Expand Up @@ -135,7 +135,15 @@ def validate(self, *args, **kwargs):
raise error

def is_type(self, instance, type):
if type not in self._types:
if type == u"object":
Copy link
Author

Choose a reason for hiding this comment

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

Perhaps we should check for object in default_types first, and fall back to this?

Copy link
Member

Choose a reason for hiding this comment

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

So, I'd rather widen the interface entirely here, rather than add complexity to the implementation.

In draft 6 for example, for some reason we decided to make 1.0 be an integer now, so doing isinstance(float) is no longer a reasonable implementation to check types.

That use case, combined with yours, makes me think that what we'll need to do now is to actually move away from the idea that types are a mapping between names and classes to check via isinstance, and that we'll have to just have e.g. Validator(is_type=SomeInjectableFunction) where the function will be passed 2 arguments, the same 2 as this is_type method. In your case for namedtuple, you'd inject in a function that first checks if it has your _asdict attribute and then calls up to the default implementation.

Does that sound reasonable?

Copy link
Author

Choose a reason for hiding this comment

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

Makes sense. I'll see what I can do.

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure you're still interested in this, but if you are, @bsmithers is also poking about in this area in a branch of his. I think he's made some progress on the track above -- if you're interested might want to sync up with him.

Gonna close this PR in the hope that we'll make enough progress there to resolve #364 but yeah this was super appreciated so definitely wouldn't mind some more hands if you are still pitching in.

if isinstance(instance, dict):
return True
elif isinstance(instance, tuple) and \
hasattr(instance.__class__, '_fields'):
return True
else:
return False
elif type not in self._types:
raise UnknownType(type, instance, self.schema)
pytypes = self._types[type]

Expand Down