Skip to content

work on more detailed messages for errors #90

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

Merged
merged 8 commits into from
Apr 19, 2013

Conversation

gazpachoking
Copy link
Contributor

Working on more detailed error messages, as mentioned in #83
Here is what they look like at the moment:

ValidationError: [{}, 3, 'foo', [3]] is not of type 'string'
    Failed validating 'type' in schema:
        {'items': {'allOf': [{'maxLength': 2, 'type': 'string'},
                             {'minimum': 5, 'type': 'integer'},
                             {'items': [{'type': 'string'},
                                        {'type': 'integer'}],
                              'type': 'array'}]},
         'type': ['string']}
    On instance:
        [{}, 3, 'foo', [3]]

ValidationError: {} is not of type 'string'
    Failed validating 'type' in schema['items']['allOf'][0]:
        {'maxLength': 2, 'type': 'string'}
    On instance[0]:
        {}

@Julian
Copy link
Member

Julian commented Apr 16, 2013

Sweet 👍.

Tests 😈?

@gazpachoking
Copy link
Contributor Author

How should tests for this sorta thing go? Just have blocks of text with the right answer and assert equal to the string we get from a ValidationError?

@Julian
Copy link
Member

Julian commented Apr 16, 2013

Yep.

Make sure there are tests that cover edge cases (empty path, empty schema path, instance with a buggy str, anything else you can think of). It's tedious. Let me know if you get tired and feel like making me fill in the rest :P.

@gazpachoking
Copy link
Contributor Author

What should we do about the different repr for strings in python 2 vs 3? (u'string' vs 'string')

@Julian
Copy link
Member

Julian commented Apr 16, 2013

Yeah I ran into this too. I'm okay I think with a hacky thing like doing
.replace("u'", "") on PY3 I think.

Didn't get a chance to look too closely yet it's late already but just from
a quick look, I think any case where the error was improperly constructed,
which I think covers any case where something is unset (correct me if I'm
wrong) should fall back on just showing less information, not sticking a
bunch of meaningless unset inside the string. We shouldn't encounter any of
these cases in practice anyhow should we?
On Apr 16, 2013 1:09 AM, "Chase Sterling" [email protected] wrote:

What should we do about the different repr for strings in python 2 vs 3? (
u'string' vs 'string')


Reply to this email directly or view it on GitHubhttps://github.com//pull/90#issuecomment-16426902
.

@Julian
Copy link
Member

Julian commented Apr 16, 2013

E.g. Fall back to just showing the message if anything is missing.
On Apr 16, 2013 1:21 AM, "Julian Berman" [email protected] wrote:

Yeah I ran into this too. I'm okay I think with a hacky thing like doing
.replace("u'", "") on PY3 I think.

Didn't get a chance to look too closely yet it's late already but just
from a quick look, I think any case where the error was improperly
constructed, which I think covers any case where something is unset
(correct me if I'm wrong) should fall back on just showing less
information, not sticking a bunch of meaningless unset inside the string.
We shouldn't encounter any of these cases in practice anyhow should we?
On Apr 16, 2013 1:09 AM, "Chase Sterling" [email protected]
wrote:

What should we do about the different repr for strings in python 2 vs 3?
(u'string' vs 'string')


Reply to this email directly or view it on GitHubhttps://github.com//pull/90#issuecomment-16426902
.

@gazpachoking
Copy link
Contributor Author

Sounds good, will do.

@gazpachoking
Copy link
Contributor Author

Well, travis seems a bit busticated at the moment, I suspect he would tell us that the python 3 pprint tests are failing right now though. Issue is that stripping the u from the string reprs make the pprint unaligned. Not sure best solution yet.

                    {u'items': {u'allOf': [{u'maxLength': 2, u'type': u'string'},
                                           {u'minimum': 5, u'type': u'integer'},

->

                    {'items': {'allOf': [{'maxLength': 2, 'type': 'string'},
                                           {'minimum': 5, 'type': 'integer'},

@Julian
Copy link
Member

Julian commented Apr 16, 2013

😦

OK. Think what you have already is great, will meditate more later if you don't come up with something beforehand. Probably will need to abandon dumb hacks :/.

Remove some debug print statements
Fix some other small errors for doctests
@bbigras
Copy link

bbigras commented Apr 18, 2013

Is it planned to have the line number with the error message so we could highlight the problems in code editors?

@Julian
Copy link
Member

Julian commented Apr 18, 2013

That'd certainly be nice, but what kind of line numbers do you have in mind? jsonschema doesn't have or swallow any extra information on line numbers, so if you wanted to do that you'd just do it the same way as usual. You have access to the instance and schema objects as provided but arbitrary objects don't know where they came from beyond what callables, modules, etc. expose.

Or do you have something specific you think we could provide that'd help?

@Julian
Copy link
Member

Julian commented Apr 19, 2013

Merged. Wahoo. Thanks!

@BrunoQC feel free to open a ticket if you have futher ideas

@Julian Julian merged commit d65af5a into python-jsonschema:master Apr 19, 2013
Julian added a commit that referenced this pull request Jun 5, 2015
3481a79 Merge pull request #91 from gelraen/nonanchoredpattern
818553f Update pattern.json
b63c96f Merge pull request #92 from gelraen/escapedref
2f043b0 Update ref.json
1ebe2b4 Add valid instances for escaped ref tests
b117902 Add a test that checks for implicit anchoring
d319afa Merge pull request #90 from bugventure/develop
a2c9de2 Add jsen to the list of validators
6d4adfe Merge pull request #87 from legoktm/protocol-relative
824cb99 Add test case for protocol-relative uri validation

git-subtree-dir: json
git-subtree-split: 3481a793ab6a1042a5973549f735b18f2355fb4e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Some new desired functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants