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

Add support for json-pointer references based on Chase Sterling's code. #37

Merged
merged 1 commit into from Oct 27, 2012

Conversation

kiall
Copy link
Contributor

@kiall kiall commented Oct 22, 2012

Hiya,

I've updated Chase Sterling's code discussed in issue #23 for the latest changes to JSONSchema. I figured I would send a PR for either you or Chase to pick up on :)

Original Source:
#23
gazpachoking@423499e

@Julian
Copy link
Member

Julian commented Oct 22, 2012

Hi :)!

When you say latest, what do you mean by that :)? Has draft 3 been revised recently here?

@kiall
Copy link
Contributor Author

kiall commented Oct 22, 2012

I had meant the latest changes to the Draft3Validator class, his branch
didn't cleanly merge with master anymore.
On Oct 22, 2012 1:50 PM, "Julian Berman" notifications@github.com wrote:

Hi :)!

When you say latest, what do you mean by that :)? Has draft 3 been revised
recently here?


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

@Julian
Copy link
Member

Julian commented Oct 22, 2012

Ah! Great, gotcha, thanks :) I'll take a look.

try:
return self._schema
except AttributeError:
return None
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason this is here? I don't think it should be necessary. Do you have a case where you got an AttributeError?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yea, L411-L415 uses it - Maybe I made have made some invalid assumptions there?

Copy link
Member

Choose a reason for hiding this comment

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

Well, self.schema is set in __init__, which means in turn that _schema should be present by the time anything wants it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Humm - Well, It's certainly possible I've made a mistake somewhere!

But - I'm fairly confident validate_ref was being called before _schema was set.

I'll re-check as soon as I can today

Copy link
Member

Choose a reason for hiding this comment

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

Oh. OK good I just wanted to check that that was the case, if it is I bet the reason why it was being called was during the meta-validation, so I think for that maybe we should just have META_SCHEMA be the default for _schema since that's going to be what's expected there.

Hopefully I'll get a chance to merge this tonight.

@Julian Julian closed this in dc4a02c Oct 26, 2012
@Julian
Copy link
Member

Julian commented Oct 26, 2012

So, thanks again for this.

After taking a quick look while cleaning this up a tiny bit, the minimum amount to make all of the tests here pass is what I have put in the json-ref branch here. I don't doubt that the unquoteing and the replaceing is necessary, but we should probably have tests that test them. Do you or @gazpachoking have examples that require them (I guess unquoting would just be one with urlencoded chars but we probably should have one of those).

@Julian Julian reopened this Oct 26, 2012
gazpachoking added a commit to gazpachoking/jsonschema that referenced this pull request Oct 26, 2012
gazpachoking added a commit to gazpachoking/jsonschema that referenced this pull request Oct 26, 2012
@gazpachoking
Copy link
Contributor

Ok, added the decoding back in along with tests. These are from section 4 and 6 of the json pointer specification

@Julian Julian merged commit 64c0d2e into python-jsonschema:master Oct 27, 2012
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.

None yet

3 participants