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 #170

Merged
merged 33 commits into from
Oct 30, 2023
Merged

Filter spatial #170

merged 33 commits into from
Oct 30, 2023

Conversation

masawdah
Copy link
Member

@masawdah masawdah commented Oct 9, 2023

Hi team,
together with @clausmichele have implemented spatial_filter process and added a test for it.

@codecov
Copy link

codecov bot commented Oct 9, 2023

Codecov Report

Merging #170 (e2f70ba) into main (8588f3d) will increase coverage by 0.11%.
Report is 9 commits behind head on main.
The diff coverage is 75.72%.

@@            Coverage Diff             @@
##             main     #170      +/-   ##
==========================================
+ Coverage   80.12%   80.24%   +0.11%     
==========================================
  Files          29       30       +1     
  Lines        1268     1382     +114     
==========================================
+ Hits         1016     1109      +93     
- Misses        252      273      +21     
Files Coverage Δ
...ses_dask/process_implementations/cubes/__init__.py 100.00% <100.00%> (ø)
...sses_dask/process_implementations/cubes/_filter.py 78.68% <80.00%> (+0.04%) ⬆️
...dask/process_implementations/cubes/mask_polygon.py 74.39% <74.39%> (ø)

... and 3 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@clausmichele clausmichele mentioned this pull request Oct 10, 2023
@clausmichele
Copy link
Member

This PR includes implementations of filter_spatial and mask_polygon. We tested them on full S2 tiles, you also may need to test them on the EODC back-end and report if you see some issues.

Current implementation supports only geoJSON vector-cubes, since currently the load_vector_cube process has not been implemted and we didn't discuss on which data structure to use for it (probably dask-geopandas?)

clausmichele added a commit to interTwin-eu/openeo-processes-dask that referenced this pull request Oct 18, 2023
@clausmichele
Copy link
Member

clausmichele commented Oct 24, 2023

@ValentinaHutter @GeraldIr can you start reviewing this PR?

We would need them for the demo at BIDS!

Copy link
Member

@GeraldIr GeraldIr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just the one question about unused variables, but that doesn't seem to impact the logic of the function, otherwise LGTM!

@clausmichele
Copy link
Member

@GeraldIr @ValentinaHutter I think it would be good to test the implementation at EODC with your cluster and with your data, to check how it behaves with large datasets.

@ValentinaHutter ValentinaHutter merged commit e63516b into Open-EO:main Oct 30, 2023
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants