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

Remove recommendation to use fully-qualified property names as queryables #31

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

philvarner
Copy link
Contributor

Related Issue(s):

Proposed Changes:

  1. Remove recommendation to use fully-qualified property names as queryables. I'm not sure why the spec ever said that, because in practice just the field names used in the properties are set as the queryables terms.

PR Checklist:

  • This PR has no breaking changes.
  • I have added my changes to the CHANGELOG or a CHANGELOG entry is not required.

Copy link
Member

@gadomski gadomski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Big 👍🏼 on this, I've bumped in to inconsistencies in implementations w.r.t. properties..

Copy link
Contributor

@m-mohr m-mohr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it should be custom queryables always, no properties.
This probably caried over from the other specs, e.g. fields, sort or query. (We should also add sortables to the sort extension).

@fmigneault
Copy link

Good thing for the removal of contradictory text. At least it is clear about the expectation.

However, I'm concerned about usability from a user's perspective. I believe the properties. prefix comes from other places that need it, such as shown in https://github.com/stac-api-extensions/fields?tab=readme-ov-file#explicitly-get-a-valid-stac-item and https://github.com/stac-api-extensions/sort?tab=readme-ov-file#http-get

It is very confusing to have a mixture of query parameters that do not operate "at the same root level" of the JSON document within the same request, although referring to the same properties position within the document. For example, filtering by eo:cloud_cover, sorting them, and removing raster:bands to reduce the content of the response is done with ?filter=eo:cloud_cover<=0.5&&sortby=+properties.eo:cloud_cover&fields=-properties.raster:bands.

Another point that is also problematic is regarding the use of filter by STAC vs OGC API - Features. In OAF, it is obvious that properties. is redundant, since all the queryable content is expected within properties, and therefore the prefix can be dropped. However, STAC's representation is somewhat different. There are assets, bands, and other "metadata containers" that could be relevant or of interest for filtering (although not currently supported by the extension). Are there plans to support such capabilities? It seems to me that using a properties. prefix would facilitate their combination in a single CQL2 filter if the capabilities were extended at a later time, rather than introducing more query parameters.

@m-mohr
Copy link
Contributor

m-mohr commented Jan 27, 2025

filter only works on queryables. Those queryable names can be chosen to be aligned with the other extensions or not, but it's up to the implementor to decide that. Sort get's something similar, sortables.

Fields is a bit different in that you need to directly tell the location of the propertes, so it's expected to be slightly different. I don't see much of an issue here, I think.

@fmigneault
Copy link

Those queryable names can be chosen to be aligned with the other extensions or not, but it's up to the implementor to decide that.

Wouldn't that make the removed recommendation "It is recommended to use fully-qualified property names (e.g., properties.eo:cloud_cover)." actually valid, provided that it is amended with an extra explanation that they should be defined as such in queryables as well?

As it stands, even unqualified properties would be invalid if they are not in queryables.

Regardless of what the recommendation says, the choice remains in the hands of the implementer, but it seems a pretty convenient approach to facilitate usage across extensions if they are fully-qualified. I don't think most users would be aware of the subtlety of fields by location vs sortby/filter by sortable/queryables when seeing similar property names within the request.

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.

4 participants