-
Couldn't load subscription status.
- Fork 42
Feature/refactor extension maps #190
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
Conversation
| itm_col_extensions_map = { | ||
| "filter": FilterExtension(client=FiltersClient()), | ||
| "pagination": TokenPaginationExtension(), | ||
| } |
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.
At first I tried to removed the duplication of the class init (e.g we do twice FilterExtension(client=FiltersClient())), but I couldn't find a nice a clear way
| # Extensions | ||
| token: Optional[str] = None, | ||
| filter_expr: Optional[str] = None, | ||
| filter_lang: Optional[str] = None, |
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.
In theory we could add sort, fields and more but this is not officially supported by the stac spec
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.
@vincentsarago I don't understand this comment. The following conformance classes exist:
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.
ahhhh this wasn't in stac-fastapi so I assume it wasn't in the spec 🤦
thanks @m-mohr
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 fixing this btw. Was just debugging why STAC Browser search queries weren't returning proper results and it seemed that there was a bug, but obviously it wasn't implemented. So good timing. I'd think supporting fields, sort and filter on the .../items endpoint would be a very welcome addition.
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.
So good timing. I'd think supporting fields, sort and filter on the .../items endpoint would be a very welcome addition.
I'll try my best to add those but we might have issues downstream for add the conformance classes in the application which will require a new stac-fastapi release
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 see, thanks. I'm fine either way as long as the conformance classes are set 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.
@m-mohr maybe you could help, there something I don't get about the conformance classes. if we have https://api.stacspec.org/v1.0.0/ogcapi-features#sort in the conformances, that doesn't mean the /collections/{collectionId}/items would have the sort parameter. It could mean that the /search endpoint support it. How do you deal with this?
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.
@vincentsarago For /search the conformance class is https://api.stacspec.org/v1.0.0/item-search#sort. For .../items it is https://api.stacspec.org/v1.0.0/ogcapi-features#sort though. So you can distinguish which endpoint supports sort. Similarly for filter etc.
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.
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 separating the different types of extensions! My main question is about the change to the list provided to the extensions argument in the StacApi definition. Does that have any downstream consequences? It would be great if you could update conftest.py with the same structure so we can be sure that setup is getting tested!
@hrodmn This PR shouldn't change what is provided to the application.
👍 |

closes #157
This PR does:
filterextension support for Item Collection endpoint