-
Notifications
You must be signed in to change notification settings - Fork 85
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
schemas: Changes to sync OGC API - Common review #953
base: master
Are you sure you want to change the base?
Conversation
It seems like the correct thing should be There's a discussion in the S/O post (which suggested |
- The main change relates to the ability to describe meaning of enum values related to opengeospatial/ogcapi-coverages#173 (comment) using a combination of `anyOf` with `const` + `title` as described in https://stackoverflow.com/questions/64233370/in-json-schema-how-to-define-an-enum-with-description-of-each-elements-in-the-e - Changes also introduce a new `x-ogc-nilValues` keyword for identifying nil values - Other changes are gramamr fixes and minor clarifications - Changing indirect dependency to QUDT in general as opposed to QUDT units since QUDT can also used for semantic definitions (not only QUDT units) - Clarified that JSON Schema keywords can also include additional keywords in section below
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See the specific comments.
On "anyOf" vs "oneOf": If used for validation, both will deliver the same result, if all options are mutually exclusive. With "oneOf" validation will take longer, but one could argue that it expresses the semantics more clearly. Anyhow, I really think we should avoid recommending the use of "anyOf" or "oneOf".
@@ -9,7 +9,7 @@ The following normative documents contain provisions that, through reference in | |||
|
|||
* [[ogc06_103r4]] Open Geospatial Consortium (OGC). OGC 06-103r4: **OpenGIS® Implementation Standard for Geographic information - Simple feature access - Part 1: Common architecture**. Edited by J. Herring. 2011. Available at http://portal.opengeospatial.org/files/?artifact_id=25355 | |||
|
|||
* [[qudtunits]] QUDT.org. **QUDT Units Vocabulary 2.1**. Edited by R. Hodgson. 2024. Available at https://www.qudt.org/doc/DOC_VOCAB-UNITS.html. | |||
* [[qudt]] QUDT.org. **QUDT Ontologies, derived models and vocabularies 2.1**. Edited by R. Hodgson. 2024. Available at https://www.qudt.org/. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change is not correct. Only the QUDT Units Vocabulary is a normative reference. The QUDT Ontologies could be added to the bibliography.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Meeting 2024-10-07: This change should be reversed. QUDT can be added to the bibliography.
@@ -12,7 +12,7 @@ The Requirements Class "Schemas" specifies basic provisions for schemas of items | |||
|Indirect Dependency |<<json-schema-validation,JSON Schema Validation: A Vocabulary for Structural Validation of JSON>> | |||
|Indirect Dependency |<<ogc06_103r4,Simple feature access - Part 1: Common architecture>> | |||
|Indirect Dependency |<<ucum,The Unified Code for Units of Measure>> | |||
|Indirect Dependency |<<qudtunits,QUDT Units Vocabulary>> | |||
|Indirect Dependency |<<qudt,QUDT Ontologies, derived models and vocabularies>> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change is not correct. Only the QUDT Units Vocabulary is normatively referenced from provisions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Meeting 2024-10-07: Same as above, this should be reversed.
^|J |Required properties SHOULD be included in "required". | ||
^|K |The JSON Schema keywords SHOULD be constrained to the those mentioned in this recommendation and requirement `/req/{req-class}/properties`. | ||
^|H |For integer properties that represent enumerated values without providing a description for each, "enum" SHOULD be provided. | ||
^|I |For integer properties that represent enumerated values while providing a description for each, "oneOf" with a combination of "const" and "title" for each entry SHOULD be provided. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using "enum" for all lists of enumerated values is easier to parse and more predictable. "oneOf" is always more problematic as one never knows how complex the different options are. And "oneOf" is often not supported in tooling.
Having descriptions for each enum could also be handled as an orthogonal capability. In ldproxy, for example, we support references to a dictionary/object that maps codes/values to titles (we call them codelists). An example: schema and a codelist.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Meeting 2024-10-07: Both options are valid and they have their drawbacks. We need more time to think about this more and work this out. @cportele will document other options.
|=== | ||
^|*Requirement {counter:req-num}* |/req/{req-class}/{req} | ||
^|A |The keyword "x-ogc-nilValues" SHALL be used to identify the values considered nil. | ||
^|B |The value of the keyword "x-ogc-nilValues" SHALL be an array of objects, each including a mandatory "const" value and an optional "title" explanation of the meaning of that particular nil value. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above. Using "enum" seems to be cleaner.
anyOf
withconst
+title
as described in https://stackoverflow.com/questions/64233370/in-json-schema-how-to-define-an-enum-with-description-of-each-elements-in-the-e ( see also v6 annotation: named enumerations json-schema-org/json-schema-spec#57 )x-ogc-nilValues
keyword for identifying nil values