stac-fastapi icon indicating copy to clipboard operation
stac-fastapi copied to clipboard

Adding free-text extension.

Open rhysrevans3 opened this issue 10 months ago • 1 comments

Related Issue(s):

Description:

Adding the free-text extension

PR Checklist:

  • [x] pre-commit hooks pass locally
  • [x] 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.

rhysrevans3 avatar Apr 10 '24 12:04 rhysrevans3

@m-mohr Could I get your opinion on this I know you've helped with the extension definition.

rhysrevans3 avatar Apr 19 '24 09:04 rhysrevans3

where are we with this PR? @rhysrevans3

vincentsarago avatar Jun 11 '24 15:06 vincentsarago

where are we with this PR? @rhysrevans3

We're unsure on if basic and advanced search are compatible https://github.com/stac-api-extensions/freetext-search/issues/10

I think the current options are, update the advanced search to better match with the basic search or separate them into two extensions.

rhysrevans3 avatar Jun 12 '24 10:06 rhysrevans3

Dear @rhysrevans3 and @m-mohr I've refactored a bit this extension to:

  • make the conformance_classes a required argument, because some implementation could only support the free-text spec in specific endpoints
  • move the stac_fastapi.extensions.core
  • added tests
  • assume that the param for both GET/POST are of string form (ref: https://github.com/stac-api-extensions/freetext-search/issues/11), I think it's then up to the implementation to parse the parameter

vincentsarago avatar Jul 18 '24 11:07 vincentsarago

  • assume that the param for both GET/POST are of string form

Correct for Advanced, not for Basic. See also https://github.com/stac-api-extensions/freetext-search/issues/11

m-mohr avatar Jul 18 '24 12:07 m-mohr

I guess I'm going to create 2 Extensions to make this a bit more clear

vincentsarago avatar Jul 18 '24 12:07 vincentsarago

That's a very good idea, especially as they are not quite compliant yet.

m-mohr avatar Jul 18 '24 12:07 m-mohr

There is a draft pr in sfeos related to this but I don't think it gives any new information - https://github.com/stac-utils/stac-fastapi-elasticsearch-opensearch/pull/227/files

jonhealy1 avatar Jul 18 '24 12:07 jonhealy1

This is ready for review

It would be nice to add documentation but this can be done in another PR ;-)

vincentsarago avatar Jul 18 '24 13:07 vincentsarago

Was the implementation tested against STAC Browser by any chance?

m-mohr avatar Jul 18 '24 15:07 m-mohr

Was the implementation tested against STAC Browser by any chance?

no, but I don't know any backend supporting the free-text spec 😓

vincentsarago avatar Jul 18 '24 15:07 vincentsarago

Was the implementation tested against STAC Browser by any chance?

@m-mohr I don't really see how the free-text extension is used by STAC-Browser

maybe https://github.com/radiantearth/stac-browser/blob/108947ebe8cbd64c7e288bbaa3c5f81293706d3e/src/components/SearchFilter.vue#L141 seems to point that q should be an array 🤷 but then I don't know if it's used in a GET or POST request

vincentsarago avatar Jul 19 '24 09:07 vincentsarago

I'm going to merge this PR (so I can continue on the collection-search which depends on the free-text ext) but feel free to comment and we could still make changes before we do a next release

vincentsarago avatar Jul 19 '24 09:07 vincentsarago