Skip to content
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

define aggregate_bbox method #203

Open
soxofaan opened this issue Apr 20, 2021 · 12 comments
Open

define aggregate_bbox method #203

soxofaan opened this issue Apr 20, 2021 · 12 comments

Comments

@soxofaan
Copy link
Member

filter_bbox expects a simple west/south/east/north dictionary
aggregate_spatial expects a shapely geometry or geojson format dict, which is less straightforward to build if you just have west/south/east/north bounds.

It could be useful for simple use cases to also provide aggregate_bbox to aggregate spatially, given a simple west/south/east/north dict

@soxofaan
Copy link
Member Author

alternative: also accept a bbox dict in aggregate_spatial and convert this to geojson at client side.

@soxofaan
Copy link
Member Author

Note: this would be a client side convenience feature and not allow UDP parameterization (because graph will still contain aggregate_spatial)

@clausmichele
Copy link
Member

I would go for using aggregate_spatial with the bbox dict, to avoid confusion with another process.

@soxofaan
Copy link
Member Author

While it pretty easy to do either of the proposed solutions in the client, we also have use cases with UDPs which could benefit from this feature. But because of the UDPs, this handling must be done at backend-side.

@m-mohr do you think it is useful to expand aggregate_spatial (in openeo-processes) to also accept a west/south/east/north bbox object instead of a geojson object? Or to define aggregate_bbox as predefined process?

@m-mohr
Copy link
Member

m-mohr commented May 17, 2021

Spontaneously: For a single bbox it's probably easier to use filter_bbox and then do a reduce_dimension, right?

@soxofaan
Copy link
Member Author

yes, but we do not support reduce_dimension over over two dimensions (x and y) at the same time, right?

@m-mohr
Copy link
Member

m-mohr commented May 17, 2021

Yes, this issue is a blocker again... If I'm remembering correctly we had a similar issue some weeks ago. Maybe we generally need to figure out a way to work on a (combined) "spatial" dimension. Need to find the other issue again....

@m-mohr
Copy link
Member

m-mohr commented May 18, 2021

I think I wouldn't add the bbox subtype to aggregate_spatial but instead try to find a solution for Open-EO/openeo-processes#226 - We need to find a common solution anyway and then filter_bbox is good enough.

@soxofaan
Copy link
Member Author

I think I wouldn't add the bbox subtype to aggregate_spatial

agreed, that's probably not the cleanest approach

another possible solution: add a process that converts a bounding box object to a geojson polygon, e.g. get_geometry as discussed here: Open-EO/openeo-processes#241 (comment)

@m-mohr
Copy link
Member

m-mohr commented May 19, 2021

For now, such a method can simply be implemented on the client-side, right?

@soxofaan
Copy link
Member Author

yeah sure, that was the original idea of this ticket. And quite trivial to implement indeed.

But the next step is also solve if for UDPs, for example where there is a single bbox parameter that has to used for both filter_bbox and aggregate_spatial . We actually have a internal use case that wants to do that.

@m-mohr
Copy link
Member

m-mohr commented May 19, 2021

But the next step is also solve if for UDPs, for example where there is a single bbox parameter that has to used for both filter_bbox and aggregate_spatial . We actually have a internal use case that wants to do that.

For a UDP that also uses aggregate_spatial in the UDP, I'd use filter_spatial instead of filter_bbox. This way you can ask for a GeoJSON as an input parameter of the UDP and then let the client convert it to GeoJSON from a given bbox.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants