search

kbgg / qgis-stac-browser

Plugin for QGIS to browse and download assets from STAC APIs
Apache License 2.0
26 stars 9 forks source link

Add filter for cloud cover #64

Closed suricactus closed 5 years ago

suricactus commented 5 years ago

I've changed the order of args of some of the internal functions, but I believe it is for good. "query" is kinda important and adding it as the last parameter seems unlogical to me, especially on 0.x version. Adding additional filters now is trivial.

There is no option for slider with two handles in QtWidgets, so we should either go to a single "max cloud coverage" slider or my solution with two spinboxes. Personally, I prefer spinbox over sliders.

kbgg commented 5 years ago

I don't believe this is the correct way to go about adding filters nor do I believe the STAC spec is mature enough to spend the effort on implementing this.

As this is implemented right now, all search queries will be searching for items which have the property eo:cloud_cover between the specified range. This poses a problem when the plugin is used to browse catalogs which are a collection of any types of items other than EO (SAR, Machine Learning, etc.). The spec doesn't specify to API developers on what to do when a search is made and an item doesn't have the property that is being filtered on (whether to include in the the results or immediately discard it).

The best way to implement this would be to fetch which properties are available in the catalogs, and create a ux flow where the user can "add" a filter to the search by selecting from the list of properties and whether to match in a range, less than, greater than, equals, etc. The user could add an arbitrary amount of filters to their query.

This brings me the the spec maturity. There is talk of adding an aggregation endpoint which would list which properties are available in the items of a catalog. There is also talk of changing the query language used for searching. Both of these reasons, to me, make it not worth the effort to implement any more extensive filtering currently.

suricactus commented 5 years ago

You are right this is a narrow use-case than has been covered here. I like the suggestion you made how to add additional filters, it perfectly makes sense and makes it very flexible. I also agree that betting on full support of filters does not worth it on the current stage.

However, in QGIS the use case would be mostly for OE datasets, at least from my perspective and the current state of the spec, qgis, community and the catalogs. Having clouldiness and date filter solves most of the GIS and RS problems, which QGIS is mostly used for. Therefore, having quick and convinient way to search for cloudless images is more than vital for the common usage, even though it's not the most flexible and "proper" way to do so. As the plugin is it's current 0.x stage, but stable enough to be used, I would suggest to keep it until something better comes. Without such filter I would still prefer to go on the sentinel/landsat websites and browse cloudless data there, rather than downloading endless list of unusable images. Having feature that solves 50% of the cases in the real world is better than a missing feature that eventually solves everything in an ideal world. Especially when the additional complexity that the half-solution brings is small and in case it does not work properly can be scrapped easily.

The change that might resolve the issue you are raising is adding a checkbox "Use cloud coverage filter"/"Use additional filters" or something similar to switch it on/off. I'm open for any other suggestion to change this if you have better ideas.

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐ On Friday, July 5, 2019 6:56 PM, Kevin Booth notifications@github.com wrote:

I don't believe this is the correct way to go about adding filters nor do I believe the STAC spec is mature enough to spend the effort on implementing this.

As this is implemented right now, all search queries will be searching for items which have the property eo:cloud_cover between the specified range. This poses a problem when the plugin is used to browse catalogs which are a collection of any types of items other than EO (SAR, Machine Learning, etc.). The spec doesn't specify to API developers on what to do when a search is made and an item doesn't have the property that is being filtered on (whether to include in the the results or immediately discard it).

The best way to implement this would be to fetch which properties are available in the catalogs, and create a ux flow where the user can "add" a filter to the search by selecting from the list of properties and whether to match in a range, less than, greater than, equals, etc. The user could add an arbitrary amount of filters to their query.

This brings me the the spec maturity. There is talk of adding an aggregation endpoint which would list which properties are available in the items of a catalog. There is also talk of changing the query language used for searching. Both of these reasons, to me, make it not worth the effort to implement any more extensive filtering currently.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

kbgg commented 5 years ago

Agreed, I think for now a checkbox to enable/disable cloud cover filtering would resolve this issue. In the future we can revisit this and add in a more advanced filtering UX.