Open-EO / openeo-processes-dask

Python implementations of many OpenEO processes, dask-friendly by default.
Apache License 2.0
19 stars 14 forks source link

Fix resample spatial #233

Closed clausmichele closed 8 months ago

clausmichele commented 8 months ago

Fix for https://github.com/Open-EO/openeo-processes-dask/issues/228

Note: I add to add the default python language in the pre-commit configuration otherwise it didn't work in my environment.

codecov[bot] commented 8 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 83.20%. Comparing base (9fcafef) to head (f363715).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #233 +/- ## ======================================= Coverage 83.20% 83.20% ======================================= Files 29 29 Lines 1536 1536 ======================================= Hits 1278 1278 Misses 258 258 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

ValentinaHutter commented 8 months ago

Thanks for checking this in the process implementation and bringing it up!

Anyhow, I have been wondering - does it make sense to rename the spatial coordinates after a reprojection in general? If data is reprojected from UTM to WGS84, the coordinates would be renamed to latitude and longitude by the reprojection and could stay like that, couldn't they?

What do you think about it?

clausmichele commented 8 months ago

The dimension labels should not be changed by this process, as specified in the process definition: https://processes.openeo.org/#resample_spatial

ValentinaHutter commented 8 months ago

Oh, right - thanks for pointing it out - it just feels a bit counter-intuitive, doesn't it? But this is then more a question on the process definition level :)

ValentinaHutter commented 8 months ago

LGTM!

clausmichele commented 8 months ago

Oh, right - thanks for pointing it out - it just feels a bit counter-intuitive, doesn't it? But this is then more a question on the process definition level :)

To me it makes sense, otherwise it could confuse the users if dimension labels change their values due to the projection/CRS you choose.