-
Notifications
You must be signed in to change notification settings - Fork 15
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
Update stac-fastapi to v2.4.8 #151
Update stac-fastapi to v2.4.8 #151
Conversation
"http://www.opengis.net/spec/ogcapi-features-3/1.0/conf/filter", | ||
"http://www.opengis.net/spec/ogcapi-features-3/1.0/conf/features-filter", | ||
"http://www.opengis.net/spec/cql2/1.0/conf/cql2-text", | ||
"http://www.opengis.net/spec/cql2/1.0/conf/cql2-json", |
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.
@philvarner added a link for cql2-json which doesn't seem to be in the stac-fastapi defaults? https://github.com/stac-utils/stac-fastapi/blob/925a9bbc686dcb1f17808719ab9863c08b92ea36/stac_fastapi/extensions/stac_fastapi/extensions/core/filter/filter.py#L73-L81
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.
that's important to have here. PC (which uses stac-fastapi) seems to have it https://planetarycomputer.microsoft.com/api/stac/v1/
That seems to be a bug, as it's defined-but-unused here https://github.com/stac-utils/stac-fastapi/blob/925a9bbc686dcb1f17808719ab9863c08b92ea36/stac_fastapi/extensions/stac_fastapi/extensions/core/filter/filter.py#L31
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.
I'd prefer to fix this upstream and then intersect with the the fix in the next release, rather than keep all of this override code just to advertise correctly.
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.
Did you use cql2-json in this project and it maybe isn't used in the other stac-fastapi versions?
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.
no, pgstac supports both also. I think it was just an oversight when the conformance classes were refactored.
In fact, pgstac's primary support is for CQL2 JSON, and the CQL Text inputs are first translated to JSON before being passed to the backend.
@@ -322,6 +324,9 @@ async def get_search( | |||
if datetime: | |||
base_args["datetime"] = datetime | |||
|
|||
if intersects: | |||
base_args["intersects"] = intersects | |||
|
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.
As far as I know we don't support intersection for GET /search. I'm not sure what an intersection query looks like in a GET request? We probably need to do something to support it. This is in the sqlalchemy implementation - https://github.com/stac-utils/stac-fastapi-sqlalchemy/blob/b09564cf78773748b13a95a9751c23270c7aa718/stac_fastapi/sqlalchemy/core.py#L271-L272
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.
It's a bug if it's not supported. The GET intersects parameter format is just a stringified GeoJSON geometry.
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.
Can you show me what this looks like? Wouldn't we have to convert the string to geojson?
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.
We at least need a test to prove that it works.
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.
the string might need to be converted to a dict[str,Any] -- I'd have to look at what type base_args is expecting for intersects.
I added issues for these things: |
Related Issue(s):
None
Description:
Update
stac-fastapi
dependencies to v2.4.8.PR Checklist:
pre-commit run --all-files
)make test
)