search

Open-EO / openeo-processes-python

A Python representation of (most) openEO processes
Apache License 2.0
11 stars 4 forks source link

Remove xarray-extras dependency #115

Open LukeWeidenwalker opened 2 years ago

LukeWeidenwalker commented 2 years ago

We only use the xarray-extras library in math.py to calculate cumulative sums and products. Couldn't we just replace this by the https://xarray.pydata.org/en/stable/generated/xarray.Dataset.cumsum.html and https://xarray.pydata.org/en/stable/generated/xarray.Dataset.cumprod.html?

I don't know what the historical reasons for this were, but getting rid of an unstable dependency like xarray-extras would be great to move towards a streamlined build process for this library! @ValentinaHutter do you see any immediate reasons why this might be a bad idea? These official functions should work with dask just fine, right?

LukeWeidenwalker commented 2 years ago

Just realized that only the exec_np version of these processes are tested, the xarray implementations aren't.

ValentinaHutter commented 2 years ago

We only use the xarray-extras library in math.py to calculate cumulative sums and products. Couldn't we just replace this by the https://xarray.pydata.org/en/stable/generated/xarray.Dataset.cumsum.html and https://xarray.pydata.org/en/stable/generated/xarray.Dataset.cumprod.html?

I don't know what the historical reasons for this were, but getting rid of an unstable dependency like xarray-extras would be great to move towards a streamlined build process for this library! @ValentinaHutter do you see any immediate reasons why this might be a bad idea? These official functions should work with dask just fine, right?

I agree, will work on the process implementation and add tests to it. You can remove the xarray-extras dependency.

ValentinaHutter commented 2 years ago

I updated the cumsum and cumproduct implementation. Now it turned out, that order and sort in the array.py also use xarray-extras functionalities. This is the reason why the tests for these processes are also failing. https://github.com/Open-EO/openeo-processes-python/issues/185

LukeWeidenwalker commented 2 years ago

Ah I see, missed those before - sort we can do with xarray.DataArray.sortby, but I don't think there's native functionality for order.

I had another look, xarray-extras doesn't resolve, because it explicitly depends on numba, which doesn't go with Python 3.11 yet. If we constrain python to be ">=3.8,<3.11", then it's fine. I think it's still a good change to replace the implementations in cumsum and cumproduct, but until we have a replacement for order, I'm happy to add this back in!