-
Notifications
You must be signed in to change notification settings - Fork 24
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
Update to STAC spec 1.0.0 #86
Update to STAC spec 1.0.0 #86
Conversation
bdf8d94
to
1826810
Compare
Here's a list of the changes to the spec with those that triggered updates in pydantic modeling called out at the end of relevant lines ("noop" when no change applied to here, "changed" when a change has been applied, and a longer note in more complicated or questionable cases): v1.0.0 - 2021-05-25
|
This is ready to be reviewed/inspected for sense but work continues to get the tests functioning correctly. Currently the majority of the tests are failing with 404s as they request 1.0.0 compliant example JSON The lack of 1.0.0 examples makes this all a bit problematic and suggestions are very welcome. |
@@ -1 +1 @@ | |||
STAC_VERSION = "1.0.0-beta.2" | |||
STAC_VERSION = "1.0.0" |
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, if omitted, allows tests to complete successfully as the prior files all appear to be 1.0.0 compliant
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.
I'm rolling some example files that simply extend the previously working versions to use some 1.0.0 features. More canonical examples can be swapped in later if that's desirable
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.
And they are assembled here, temporarily: https://gist.github.com/moradology/977878279e1f36a6494877cdeaf18688
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.
I've gone ahead and folded the testing files within this repository
{ | ||
"stac_version": "1.0.0", | ||
"stac_extensions": [ | ||
"scientific", |
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.
The extension ids need to be fully qualified in 1.0.0 I think?
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.
You're absolutely right
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.
I'm going to fix this in the test files but I wonder if more validation isn't desirable in the model itself to avoid this kind of oversight
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.
It looks like JSON Schema now requires it to be an iri
https://github.com/radiantearth/stac-spec/blob/789e90fba4c7119bc925d26f75f47bacf44c91e6/item-spec/json-schema/item.json#L105
So maybe use AnyUrl
or similar? https://pydantic-docs.helpmanual.io/usage/types/#urls
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.
Looks great! Some minor comments.
Codecov Report
@@ Coverage Diff @@
## master #86 +/- ##
==========================================
- Coverage 98.33% 96.38% -1.96%
==========================================
Files 33 19 -14
Lines 779 442 -337
==========================================
- Hits 766 426 -340
- Misses 13 16 +3
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
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.
@moradology This looks great!
This PR is aimed at updating models to correspond to updates in the STAC spec since 1.0.0-beta. Changes introduced here are based on the relevant updates to STAC-Spec listed here in rc1, rc2, rc3, rc4, and 1.0.0: https://github.com/radiantearth/stac-spec/blob/v1.0.0/CHANGELOG.md