-
Notifications
You must be signed in to change notification settings - Fork 180
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
Integrate item_assets field from extension into core Collection spec #1289
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.
stac-extensions/item-assets#4 must be clarified before we should merge it into core, I think.
"required": [ | ||
"title", | ||
"type" | ||
], |
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 is not reflected in the Markdown. I think it only says that any two properties are required (minProperties: 2
). Maybe this was already wrong in the original spec, but should be fixed here.
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.
Dont you think it is better to reflect in the markdown that title and type are required?
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.
Is that the intention? I thought the intention is to require at least two properites. I think I'd replace the required fields with minProperties: 2... title is not even always available. See https://github.com/radiantearth/stac-spec/pull/1289/files#r1668474173
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.
Any assumptions about this in pgstac, @bitner or @vincentsarago?
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.
FWIW I checked pystac in the STAC community meeting yesterday and didn't see any baked-in expectations around this.
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.
To align better with stac-extensions/item-assets#4 and to make the terminology a bit more clear.
Co-authored-by: Matthias Mohr <[email protected]>
Co-authored-by: Matthias Mohr <[email protected]>
Co-authored-by: Matthias Mohr <[email protected]>
Co-authored-by: Matthias Mohr <[email protected]>
Co-authored-by: Matthias Mohr <[email protected]>
Co-authored-by: Matthias Mohr <[email protected]>
Co-authored-by: Matthias Mohr <[email protected]>
Related Issue(s): #1275
Proposed Changes:
item_assets
field in Collections are integrated from extension into the core Collection spec. (#1275)PR Checklist:
or a CHANGELOG entry is not required.
and I have opened issue/PR #XXX to track the change.