-
Notifications
You must be signed in to change notification settings - Fork 48
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
Collection search #735
Collection search #735
Conversation
I added the |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #735 +/- ##
==========================================
+ Coverage 93.43% 93.68% +0.25%
==========================================
Files 13 15 +2
Lines 990 1188 +198
==========================================
+ Hits 925 1113 +188
- Misses 65 75 +10 ☔ View full report in Codecov by Sentry. |
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.
Thanks for this! I did an initial pass and left a few comments. I'll do a more thoughtful review later.
fa25492
to
fee6c97
Compare
I think the last big thing to add here is a |
Yup, makes sense to me! |
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.
Taking a look at the CI errors, looks like you'll need to use pytest.warns to catch-and-assert the client-side filtering warnings.
Argh, yeah. I need to start running Thanks for the review, I'll get those changes in today! |
or develop an allergic reaction to all warnings, like I have (don't recommend leads to lots of yak shaving) :-) |
ee88cef
to
faa8d8c
Compare
1457fbb
to
fce4bd0
Compare
@gadomski thanks for your reviews, sorry for not catching those little CI issues and for half-accepting your suggestion on |
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! Only thing missing is a CHANGELOG entry. Thanks for the iterations @hrodmn!
Related Issue(s):
Description:
Collection discovery is a challenge in the current environment. A user might know which catalog or API they want data from but they do not know have the actual
collection_id
that they will need to perform an item-level search. The STAC API Collection Search Extension makes it possible for a user to search apply filters to collection-level metadata. This is most useful when the STAC API Free Text Extension is enabled because a user can search an API for all collections with a term likeq=DEM
to find all collections that have the term DEM in the title, description, or keywords.Since most APIs do not currently have the collections earch extension enabled, I added some client-side filtering logic to make the
CollectionSearch
class request the full list of collections from the/collections
endpoint then apply a limited set of filters (datetime
,bbox
,q
) to the list.ItemSearch
class to inherit from a newBaseSearch
class so methods can be shared betweenItemSearch
andCollectionSearch
classesCollectionSearch
classClient.collection_search
methodcli.py
CollectionSearch
Client.collection_search
test_cli.py
PR Checklist: