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

Specify draft via $schema in JSON schema #153

Merged
merged 2 commits into from Aug 11, 2018

Conversation

dhimmel
Copy link
Contributor

@dhimmel dhimmel commented Aug 7, 2018

The goal is to allow validators to detect which draft to use, without having to hardcode it in downstream applications. For example, I want my application to use the latest CSL Items schema and to continue working even if this repository upgrades to a newer schema draft.

@dhimmel dhimmel changed the title Specify JSON Schema draft in csl-data.json Specify draft via $schema in JSON schema Aug 7, 2018
@rmzelle
Copy link
Member

rmzelle commented Aug 9, 2018

Can you point to some documentation that this is the proper way to indicate the draft version?

I want my application to use the latest CSL Items schema and to continue working even if this repository upgrades to a newer schema draft.

While indicating the draft version would obviously be a good thing, we might want to make sure we also apply this change in the release branches, instead of just in master, which we use for development.

@dhimmel
Copy link
Contributor Author

dhimmel commented Aug 9, 2018

Can you point to some documentation that this is the proper way to indicate the draft version?

Disclaimer, I'm not an expert on JSON Schemas (the CSL one is the first I've ever used). This is how I saw it done in several schemas as well as the jsonschema python parser. Also this is what the official draft 3 meta schema uses.

Here's the documentation of $schema from the IETF standard:

This attribute defines a URI of a JSON Schema that is the schema of the current schema. When this attribute is defined, a validator SHOULD use the schema referenced by the value's URI (if known and available) when resolving Hyper Schema (Section 6) links (Section 6.1).

A validator MAY use this attribute's value to determine which version of JSON Schema the current schema is written in, and provide the appropriate validation features and behavior. Therefore, it is RECOMMENDED that all schema authors include this attribute in their schemas to prevent conflicts with future JSON Schema specification changes.

Regarding

we might want to make sure we also apply this change in the release branches, instead of just in master, which we use for development.

I think you mean release tags (not branches), as I don't see any release branches in this repo. I would think it's best not to change a release once it's been released, but to release a new patch number as per semantic versioning. For example, our changes could go into release 1.0.2 or perhaps 1.1.0 (if they warrant a minor release).

@rmzelle
Copy link
Member

rmzelle commented Aug 9, 2018

I think you mean release tags (not branches), as I don't see any release branches in this repo. I would think it's best not to change a release once it's been released, but to release a new patch number as per semantic versioning. For example, our changes could go into release 1.0.2 or perhaps 1.1.0 (if they warrant a minor release).

Ah, right (we do have branches at https://github.com/citation-style-language/documentation for the various CSL releases).

We might want to re-license the original CSL 0.8.1, 1.0, and 1.0.1 schemas, though, instead of just future releases, so we might want to create branches at the current tags (and retag the tips of those branches).

dhimmel added a commit to dhimmel/csl-schema that referenced this pull request Aug 10, 2018
@dhimmel
Copy link
Contributor Author

dhimmel commented Aug 10, 2018

I'd recommend a branch for each MAJOR.MINOR combination that is still receiving new PATCH releases. I wouldn't change a tag to refer to a different commit once it has been published, as that could be destructive to some users who expect a certain release to never change. So for example, you would create a 1.0 branch, which would take commits from master that should be released in the next PATCH. Then at some point when ready to release the PATCH, you would tag a commit on the 1.0 branch as 1.0.2 (or whichever PATCH number is next in sequence).

At this point, I'm not sure how critical this is, as it can always be done later.

@rmzelle rmzelle merged commit 04504e8 into citation-style-language:master Aug 11, 2018
@rmzelle
Copy link
Member

rmzelle commented Aug 11, 2018

Thanks! (we have an open issue for upgrading these schemas to draft 4, by the way, although it looks like JSON schema is currently already at draft 7; #127)

I wouldn't change a tag to refer to a different commit once it has been published, as that could be destructive to some users who expect a certain release to never change.

Well, historically we never patched older versions of the schema. I would only retag here to change the schema license of the older releases. (for the "documentation" repo we often have commits after a release to fix typos, formatting (we moved to Read the Docs after the most recent CSL release), and broken links.

@dhimmel dhimmel deleted the patch-1 branch August 11, 2018 21:52
@dsifford
Copy link

dsifford commented Dec 2, 2018

The schema isn't even valid draft-3 to my knowledge. On every v3 validator I've tried, it errors because you're using $ref to point at non-absolute reference paths...

In other words....

This

schema/csl-data.json

Lines 132 to 137 in 15c3185

"collection-editor": {
"type": "array",
"items": {
"$ref": "name-variable"
}
},

Should be this

"collection-editor": {
    "type": "array",
    "items": {
        "$ref": "#/items/properties/author/items" 
    } 
}

And this

schema/csl-data.json

Lines 255 to 257 in 15c3185

"container": {
"$ref": "date-variable"
},

Should be this

"container": {
    "$ref": "#/items/properties/accessed" 
}
Here's the valid JSON schema for csl-data.json
{
    "description": "JSON schema for CSL input data",
    "$schema" : "http://json-schema.org/draft-03/schema#",
    "id": "https://github.com/citation-style-language/schema/raw/master/csl-data.json",
    "type": "array",
    "items": {
        "type": "object",
        "properties": {
            "type": {
                "type": "string",
                "required": true,
                "enum" : [
                    "article",
                    "article-journal",
                    "article-magazine",
                    "article-newspaper",
                    "bill",
                    "book",
                    "broadcast",
                    "chapter",
                    "dataset",
                    "entry",
                    "entry-dictionary",
                    "entry-encyclopedia",
                    "figure",
                    "graphic",
                    "interview",
                    "legal_case",
                    "legislation",
                    "manuscript",
                    "map",
                    "motion_picture",
                    "musical_score",
                    "pamphlet",
                    "paper-conference",
                    "patent",
                    "personal_communication",
                    "post",
                    "post-weblog",
                    "report",
                    "review",
                    "review-book",
                    "song",
                    "speech",
                    "thesis",
                    "treaty",
                    "webpage" 
                ] 
            },
            "id": {
                "type": [
                    "string",
                    "number" 
                ],
                "required": true 
            },
            "categories": {
                "type": "array",
                "items": {
                    "type": "string" 
                } 
            },
            "language": {
                "type": "string" 
            },
            "journalAbbreviation": {
                "type": "string" 
            },
            "shortTitle": {
                "type": "string" 
            },
            "author": {
                "type": "array",
                "items": {
                    "id": "name-variable",
                    "type": [
                        {
                            "properties": {
                                "family" : {
                                    "type": "string" 
                                },
                                "given" : {
                                    "type": "string" 
                                },
                                "dropping-particle" : {
                                    "type": "string" 
                                },
                                "non-dropping-particle" : {
                                    "type": "string" 
                                },
                                "suffix" : {
                                    "type": "string" 
                                },
                                "comma-suffix" : {
                                    "type": [
                                        "string",
                                        "number",
                                        "boolean" 
                                    ] 
                                },
                                "static-ordering" : {
                                    "type": [
                                        "string",
                                        "number",
                                        "boolean" 
                                    ] 
                                },
                                "literal" : {
                                    "type": "string" 
                                },
                                "parse-names" : {
                                    "type": [
                                        "string",
                                        "number",
                                        "boolean" 
                                    ] 
                                } 
                            },
                            "additionalProperties" : false 
                        },
                        {
                            "properties": {
                                "literal" : {
                                    "type": "string" 
                                } 
                            },
                            "additionalProperties" : false 
                        } 
                    ] 
                } 
            },
            "collection-editor": {
                "type": "array",
                "items": {
                    "$ref": "#/items/properties/author/items" 
                } 
            },
            "composer": {
                "type": "array",
                "items": {
                    "$ref": "#/items/properties/author/items" 
                } 
            },
            "container-author": {
                "type": "array",
                "items": {
                    "$ref": "#/items/properties/author/items" 
                } 
            },
            "director": {
                "type": "array",
                "items": {
                    "$ref": "#/items/properties/author/items" 
                } 
            },
            "editor": {
                "type": "array",
                "items": {
                    "$ref": "#/items/properties/author/items" 
                } 
            },
            "editorial-director": {
                "type": "array",
                "items": {
                    "$ref": "#/items/properties/author/items" 
                } 
            },
            "interviewer": {
                "type": "array",
                "items": {
                    "$ref": "#/items/properties/author/items" 
                } 
            },
            "illustrator": {
                "type": "array",
                "items": {
                    "$ref": "#/items/properties/author/items" 
                } 
            },
            "original-author": {
                "type": "array",
                "items": {
                    "$ref": "#/items/properties/author/items" 
                } 
            },
            "recipient": {
                "type": "array",
                "items": {
                    "$ref": "#/items/properties/author/items" 
                } 
            },
            "reviewed-author": {
                "type": "array",
                "items": {
                    "$ref": "#/items/properties/author/items" 
                } 
            },
            "translator": {
                "type": "array",
                "items": {
                    "$ref": "#/items/properties/author/items" 
                } 
            },
            "accessed": {
                "id": "date-variable",
                "type": [
                    {
                        "properties": {
                            "date-parts": {
                                "type": "array",
                                "items": {
                                    "type": "array",
                                    "items": {
                                        "type": [
                                            "string",
                                            "number" 
                                        ] 
                                    },
                                    "maxItems": 3 
                                },
                                "maxItems": 2 
                            },
                            "season": {
                                "type": [
                                    "string",
                                    "number" 
                                ] 
                            },
                            "circa": {
                                "type": [
                                    "string",
                                    "number",
                                    "boolean" 
                                ] 
                            },
                            "literal": {
                                "type": "string" 
                            },
                            "raw": {
                                "type": "string" 
                            }
                        },
                        "additionalProperties" : false 
                    },
                    {
                        "properties": {
                            "literal": {
                                "type": "string" 
                            } 
                        },
                        "additionalProperties" : false 
                    } 
                ] 
            },
            "container": {
                "$ref": "#/items/properties/accessed" 
            },
            "event-date": {
                "$ref": "#/items/properties/accessed" 
            },
            "issued": {
                "$ref": "#/items/properties/accessed" 
            },
            "original-date": {
                "$ref": "#/items/properties/accessed" 
            },
            "submitted": {
                "$ref": "#/items/properties/accessed" 
            },
            "abstract": {
                "type": "string" 
            },
            "annote": {
                "type": "string" 
            },
            "archive": {
                "type": "string" 
            },
            "archive_location": {
                "type": "string" 
            },
            "archive-place": {
                "type": "string" 
            },
            "authority": {
                "type": "string" 
            },
            "call-number": {
                "type": "string" 
            },
            "chapter-number": {
                "type": "string" 
            },
            "citation-number": {
                "type": "string" 
            },
            "citation-label": {
                "type": "string" 
            },
            "collection-number": {
                "type": "string" 
            },
            "collection-title": {
                "type": "string" 
            },
            "container-title": {
                "type": "string" 
            },
            "container-title-short": {
                "type": "string" 
            },
            "dimensions": {
                "type": "string" 
            },
            "DOI": {
                "type": "string" 
            },
            "edition": {
                "type": [
                    "string",
                    "number" 
                ] 
            },
            "event": {
                "type": "string" 
            },
            "event-place": {
                "type": "string" 
            },
            "first-reference-note-number": {
                "type": "string" 
            },
            "genre": {
                "type": "string" 
            },
            "ISBN": {
                "type": "string" 
            },
            "ISSN": {
                "type": "string" 
            },
            "issue": {
                "type": [
                    "string",
                    "number" 
                ] 
            },
            "jurisdiction": {
                "type": "string" 
            },
            "keyword": {
                "type": "string" 
            },
            "locator": {
                "type": "string" 
            },
            "medium": {
                "type": "string" 
            },
            "note": {
                "type": "string" 
            },
            "number": {
                "type": [
                    "string",
                    "number" 
                ] 
            },
            "number-of-pages": {
                "type": "string" 
            },
            "number-of-volumes": {
                "type": [
                    "string",
                    "number" 
                ] 
            },
            "original-publisher": {
                "type": "string" 
            },
            "original-publisher-place": {
                "type": "string" 
            },
            "original-title": {
                "type": "string" 
            },
            "page": {
                "type": "string" 
            },
            "page-first": {
                "type": "string" 
            },
            "PMCID": {
                "type": "string" 
            },
            "PMID": {
                "type": "string" 
            },
            "publisher": {
                "type": "string" 
            },
            "publisher-place": {
                "type": "string" 
            },
            "references": {
                "type": "string" 
            },
            "reviewed-title": {
                "type": "string" 
            },
            "scale": {
                "type": "string" 
            },
            "section": {
                "type": "string" 
            },
            "source": {
                "type": "string" 
            },
            "status": {
                "type": "string" 
            },
            "title": {
                "type": "string" 
            },
            "title-short": {
                "type": "string" 
            },
            "URL": {
                "type": "string" 
            },
            "version": {
                "type": "string" 
            },
            "volume": {
                "type": [
                    "string",
                    "number" 
                ] 
            },
            "year-suffix": {
                "type": "string" 
            } 
        },
        "additionalProperties" : false 
    } 
}

Instead of just fixing that though, can we just update this? JSON schema is on draft 7 now...

@dhimmel
Copy link
Contributor Author

dhimmel commented Dec 3, 2018

On every v3 validator I've tried, it errors because you're using $ref to point at non-absolute reference paths...

Hmm I wonder if this is what caused python-jsonschema/jsonschema#447.

JSON schema is on draft 7 now...

Does draft 7 support this form of non-absolute reference path?

P.S. I don't want to delve too deep into upgrading the draft version in this issue, since that is off topic (not that this shouldn't be discussed, just not here).

Here's the valid JSON schema for csl-data.json

What to open a pull request with the fixes? I'm not a maintainer of this repo, which is not very actively maintained, but some simple PRs seem to get accepted. Therefore, we've been maintaining a fork which we use in Manubot. We would certainly be happy to cherry-pick this commit, if you create it!

@rmzelle
Copy link
Member

rmzelle commented Dec 3, 2018

We'd be more than happy to work with you to clean up this schema and upgrade it to the latest draft, via whatever path makes more sense (first fixing the outstanding issues in the current v3 draft, and then upgrade to v7, or vice versa).

Ideally we also would have Travis-CI verify that the schemas validate and properly describe the test suite CSL JSON snippets at https://github.com/citation-style-language/test-suite (which are embedded in the tests, and, since we only validated the snippets against these JSON schemas when we first wrote the schemas, some are likely to have a few validation errors in them).

@dsifford
Copy link

dsifford commented Dec 3, 2018

Hmm I wonder if this is what caused python-jsonschema/jsonschema#447.

Yes, it is for sure.

Does draft 7 support this form of non-absolute reference path?

Not sure (I'm certainly not a JSON schema pro by any stretch of the imagination) but I do know that each iteration of the schema version seemed to pepper in more ergonomic features from just glancing at the highlights.

We'd be more than happy to work with you to clean up this schema and upgrade it to the latest draft, via whatever path makes more sense (first fixing the outstanding issues in the current v3 draft, and then upgrade to v7, or vice versa).

Cool. Happy to band together on this as time allows.

Ideally we also would have Travis-CI verify that the schemas validate and properly describe the test suite CSL JSON snippets.

Yeah, that sounds like a sorely needed feature of the test suite for sure. Also happy to band together on that as well once we can get this fleshed out.

@dsifford
Copy link

dsifford commented Dec 3, 2018

@rmzelle can you go through and either merge or close the open PRs before I start on this? Want to make sure that if we do update the schemas, the most updated and accurate info is included.

@rmzelle
Copy link
Member

rmzelle commented Dec 3, 2018

@dsifford
Copy link

dsifford commented Dec 3, 2018

Any outstanding... I see there's PRs back into 2015 even. Is it worth keeping those open?

@rmzelle
Copy link
Member

rmzelle commented Dec 3, 2018

Several relate to the actual design of the CSL language, and those are certainly relevant and will be addressed once we get some momentum for the next CSL release. Most of these should be issues instead of pull requests, since the eventual implementation will likely differ significantly from what the requestors are asking for, but we can leave them as is.

And you're most concerned with PRs in the schema repo, right?

@dsifford
Copy link

dsifford commented Dec 3, 2018

Yep.. Just want to make sure the current existing JSON schema is correct before diving in too far.

@dsifford
Copy link

dsifford commented Dec 3, 2018

@dhimmel See here: https://json-schema.org/understanding-json-schema/structuring.html#using-id-with-ref

Specifically:

Note: This functionality isn’t currently supported by the Python jsonschema library.

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

3 participants