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

filter_spatial vs filter_vector #460

Closed
soxofaan opened this issue Aug 24, 2023 · 1 comment · Fixed by #462
Closed

filter_spatial vs filter_vector #460

soxofaan opened this issue Aug 24, 2023 · 1 comment · Fixed by #462
Milestone

Comments

@soxofaan
Copy link
Member

Not sure if there is still room to change a lot here, but while looking into vector cube issues I observed this:

  • filter_bbox supports both raster and vector cubes
  • filter_spatial supports only raster cubes and filter_vector support only vector cubes, but both work almost identical (assuming relation="intersect" in filter_vector). Is there some opportunity to unify this better, e.g. allow vector cubes in filter_spatial?

I think at least we should have better cross-referencing (documentation-wise) between these processes (there is no documentation link between filter_spatial and filter_vector)

@m-mohr
Copy link
Member

m-mohr commented Sep 5, 2023

I considered it, but based on the relation parameter I found it akward.

You'd need to assume the Pixels are points at the pixel center, but I'm not sure whether you'd want to implement all the allowed relation types that you have for vectors also for rasters. What happens if you only support intersection for raster and all other relation types for vector? Confusing. filter_bbox was merged as it doesn't support the relation types and is much simpler in that regard.

But indeed, crosslinking between them would be good.

@m-mohr m-mohr added this to the 2.0.0 milestone Sep 5, 2023
soxofaan added a commit to soxofaan/openeo-processes that referenced this issue Sep 5, 2023
m-mohr pushed a commit that referenced this issue Sep 30, 2023
@m-mohr m-mohr closed this as completed Sep 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants