Skip to content
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

Align fields extension behavior with extension suggestions #527

Closed
wants to merge 4 commits into from

Conversation

gadomski
Copy link
Member

@gadomski gadomski commented Feb 13, 2023

Related Issue(s):

This is one of two PRs that would fix stac-utils/stac-fastapi-pgstac#27; the other is #524. We should merge one and close the other (should happen automatically thanks to closing keywords).

Description:

This PR updates both backends to (mostly) follow the recommended field extension semantics, as laid out in https://github.com/stac-api-extensions/fields#includeexclude-semantics. In particular:

  • Any include fields are merged with the default include set
  • An empty fields attribute will return the default include set
  • If a field is both included and excluded, it is included in the response (this is a change from previous behavior)
  • We include stac_version in the default includes to ensure valid STAC items
  • In a deviation from the recommended semantics, we keep collection and stac_extensions, as (in this author's opinion) these are valuable and cheap

The recommendations are encapsulated in a into_recommended method, where the set operations are performed: https://github.com/stac-utils/stac-fastapi/compare/issues/395-align-include-with-suggestions?expand=1#diff-b9d01a5a656ba2bdee8dd6eb5462bf1b1602b01bceaafb431cff8c38969815d5R61-R76.

I've also updated the fields extension default to be None, which allows for detection of an empty fields attribute (e.g. ?fields=). This was required to enable item 1 in https://github.com/stac-api-extensions/fields#includeexclude-semantics.

PR Checklist:

  • Code is formatted and linted (run pre-commit run --all-files)
  • Tests pass (run make test)
  • Documentation has been updated to reflect changes, if applicable, and docs build successfully (run make docs)
  • Changes are added to the CHANGELOG.

This allows detection of whether `fields` was present in the request at all.
@gadomski gadomski added this to the 2.4.4 milestone Feb 13, 2023
@gadomski gadomski self-assigned this Feb 13, 2023
@mmcfarland
Copy link
Contributor

I understand the rationale for making it easy to always return a valid STAC Item despite what you put in include, and it makes sense to adhere to spec suggestions so that users get similar behavior between implementations. However, I have workflows where all I really care about is the ID (or maybe ID + datetime), and I'll just include that attribute in include in order to get the smallest possible payload (which can save a considerable amount of size/time when there are a lot of assets and a large result count).

In that case, I understand I shouldn't try to parse the results as Items. I also think it's a bit cumbersome to have to copy over fields I would now see in my results to exclude, and many users who don't read the spec would be unlikely to assume they could even do that. That said, my use case is an optimization and having default valid STAC items is less likely to break anything. I'm moderately in favor of keeping the intuitive behavior (sans links) from pgstac backend.

@philvarner
Copy link
Collaborator

Additionally, the Field Extension explicitly says that it makes returning invalid Items legal, so it's a desire to have valid items returned rather than a requirement.

@gadomski
Copy link
Member Author

Converting to draft until stac-api-extensions/fields#5 lands. Once it does, I think we'll use #524 as the base solution, but there could be ideas from this PR that would be useful to port over.

@gadomski
Copy link
Member Author

gadomski commented Mar 7, 2023

Closing as blocked by stac-api-extensions/fields#5.

@gadomski gadomski closed this Mar 7, 2023
@gadomski gadomski deleted the issues/395-align-include-with-suggestions branch May 9, 2023 18:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PgSTAC: Fields extension includes links even if not listed in includes
3 participants