-
Notifications
You must be signed in to change notification settings - Fork 2
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
feat: add summaries to historical imagery related extensions #176
Conversation
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.
lgtm
You're right, we have raised an issue on the STAC spec about summarising assets. |
|
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 good to me. Just a tinny suggestion concerning the title
which some don't start with a capital letter.
extensions/scanning/schema.json
Outdated
"required": ["minimum", "maximum"], | ||
"properties": { | ||
"minimum": { | ||
"title": "earliest date", |
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
extensions/scanning/schema.json
Outdated
"pattern": "(\\+00:00|Z)$" | ||
}, | ||
"maximum": { | ||
"title": "most recent date", |
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
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.
Just two title
which do not start with a Capital letter. Only esthetic/consitency with other title
.
6d03490
c62d9e1
to
6d03490
Compare
6d03490
to
ccc9a81
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.
Looks good
Extensions updated to include summary requirements:
nb: eo:bands cannot be added to the historical imagery schema as it would be an asset summary (please correct me if I'm wrong here).