-
Notifications
You must be signed in to change notification settings - Fork 178
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
JSON Schema in summaries #1045 #1093
Conversation
c38721a
to
e2f5c49
Compare
@jjrom Could you please have a look at this PR? I had to derive a bit from our discussion (see the issue for some context). I'm happy to discuss this again in a short call, too. |
e2f5c49
to
6755d41
Compare
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.
Thanks @m-mohr for this PR ! Looks good to use json-schema in summaries.
I have 2 remarks :
- I would suggest to add an example of a summaries property including an array of object with the const keyword (as you provided here Extend summary to support list of objects / Better describe summary values #1045 (comment)).
- In https://github.com/radiantearth/stac-spec/blob/6755d41c68f45cdcbf908b20c91c101fce685f2a/collection-spec/collection-spec.md#json-schema-object the sentence "For a full understanding of the summarized field, a JSON Schema can be added for each summarized field" is not very clear. We should be more explicit with a focus on a real use case (the count use case for instance ?)
@jjrom Do you have an example at hand that fits into the examples folder? Could be an old one that we convert to the new schema. |
Perhaps we could duplicate the "collection.json" example (https://github.com/radiantearth/stac-spec/blob/master/examples/collection.json) to a "collection-extended-summaries.json" example in which we replace
with
? |
@jjrom I think we can only add it to the collection-only folder right now. The others are a full catalog, so the count would likely be 1 or 2 as there are not so many items and we also can't add two collections, because then the items would need to link to two of them. Maybe we could duplicate, shorten and then convert https://github.com/radiantearth/stac-spec/blob/master/examples/collection-only/collection.json a bit and also add titles to epsg codes as you mentioned in the call? |
@m-mohr Ok that makes sense. In this case we can duplicate https://github.com/radiantearth/stac-spec/blob/master/examples/collection-only/collection.json and update the platform property. Concerning the EPSG codes, I'm fine to add titles but only if we shorten massively the property content - for instance limits it to two EPSG codes (e.g. EPSG:4326 and EPSG:3857). Otherwise it would be hard to read |
Yes, agreed! Or can you export your "best" example from your implementation, which we then try to convert to the new format? Would be a good verification that the new schema works well with your data. |
I plan to update my implementation to fit the new schema next week. If it's not too late I can provide a real example by the end of next week. Otherwise we stick to the existing collection.json duplication/shortening as we discussed (?) |
@jjrom I thought about an example from your current implementation. That should be easy enough to migrate "manually". |
+1 on 'manual migration'. I have some time today and can try to help. |
@schwehr This looks very much like it could be useful for you as you have this gee:schema thing in your catalogs, right? Any thought on the PR? Could this be useful for you, too? |
Some initial thoughts while reading to catch up... I'm not familiar enough with jsonschema. And I haven't had a chance to talk it over with simonf.
|
The CI fails due to stac-extensions/scientific#2. Once the PR is merged, we need to release scientific as 1.0.1 and change the schema URLs here / in stac-spec. |
I'm wondering whether we should mark this new feature 'experimental' (with the chance of breaking in a minor release)? |
Put in a suggestion for units: json-schema-org/json-schema-vocabularies#46 |
Is there a way to make the feature an extension? Happy for it to not be namespaced and even mentioned in the main spec. But that could let us evolve it without having to cut stac-spec releases. |
No, it's not really extensible, I'd say. At least the extension would have the potential to break implementations and thus it's not a good extension. |
Thanks, @schwehr.
Not standardized, but custom keywords are a thing.
Not sure what exactly you are asking for?
We encourage to specify what values to expect to make it more useful for users. Just specifying that the field exists (with a specific data type?) would not be a best practice IMHO. Ideally, there would be more information.
I'm not sure STAC is the right place to go into JSON Schema details. Wouldn't it be better to just link to https://json-schema.org/learn/ ?
There's no direct type value for typed arrays in JSON Schema, instead you do something like:
JSON Schema allows for custom keywords, so I'm not sure there's a need for another annotation in a validation language as you can simply define it on your own. But let's see what they think. |
Is a custom type available within the actual schema being defined inside the STAC summary? In the case I'm talking about, the data isn't what's going to have the units. The schema says what the units are going to be.
There is little sense of the flexibility of the possibilities here. I agree that the reader should go look at the json schema spec, but the examples here are extremely narrow.
Examples of non-categorical data:
|
remove scientific extension
Co-authored-by: Matthias Mohr <m.mohr@uni-muenster.de>
This seems good for now. We can fine-tune docs and examples later. |
Related Issue(s): json-schema-org/json-schema-spec#1045
Proposed Changes:
PR Checklist: