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

Add basic handling for X,Y,band,time dimensions for transpose and rep… #244

Merged
merged 4 commits into from
May 6, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@ A subset of process implementations with heavy or unstable dependencies are hidd
## Development environment
openeo-processes-dask requires poetry >1.2, see their [docs](https://python-poetry.org/docs/#installation) for installation instructions.

Clone the repository with `--recurse-submodules` to also fetch the process specs:
```
git clone --recurse-submodules git@github.com:Open-EO/openeo-processes-dask.git
```

To setup the python venv and install this project into it run:
```
poetry install --all-extras
Expand Down
45 changes: 37 additions & 8 deletions openeo_processes_dask/process_implementations/cubes/resample.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import logging
from typing import Optional, Union
from typing import Optional, Union, Literal

import odc.geo.xr
import rioxarray # needs to be imported to set .rio accessor on xarray objects.
from odc.geo.geobox import resolution_from_affine
from odc.geo.geobox import resolution_from_affine, AnchorEnum, XY
from pyproj.crs import CRS, CRSError

from openeo_processes_dask.process_implementations.data_model import RasterCube
Expand Down Expand Up @@ -37,9 +37,12 @@ def resample_spatial(
projection: Optional[Union[str, int]] = None,
resolution: int = 0,
method: str = "near",
align: str = "upper-left",
):
"""Resamples the spatial dimensions (x,y) of the data cube to a specified resolution and/or warps the data cube to the target projection. At least resolution or projection must be specified."""

#TODO: Actually implement align parameter in some way

# Assert resampling method is correct.
if method == "near":
method = "nearest"
Expand All @@ -49,14 +52,40 @@ def resample_spatial(
f'Selected resampling method "{method}" is not available! Please select one of '
f"[{', '.join(resample_methods_list)}]"
)

dims = data.dims
if dims is None:
raise OpenEOException(
f"Data cube has no dimensions"
)

# Re-order, this is specifically done for odc reproject
data_cp = data.transpose(
data.openeo.band_dims[0],
data.openeo.temporal_dims[0],
data.openeo.y_dim,
data.openeo.x_dim,
try:
if 'band' in dims and 'time' in dims and 'y' in dims and 'x' in dims:
# all dimensions present
data_cp = data.transpose('band', 'time', 'y', 'x')
elif 'time' in dims and 'y' in dims and 'x' in dims:
# no band dimension
data_cp = data.transpose('time', 'y', 'x')
elif 'band' in dims and 'y' in dims and 'x' in dims:
# no time dimension
data_cp = data.transpose('band', 'y', 'x')
else:
# no band and time dimension
data_cp = data.transpose('y', 'x')
except Exception:
raise OpenEOException(
f"Data cube has unexpected dimensions: {dims}"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for your inputs on the process and adding the align parameter!
I think, we can keep using the data.openeo here, as this checks the dimensions in the dataset - and for example uses t instead of time if the temporal dimension is called like that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi, sure but using data.openeo limits you to using band , time, x and y only now no? Because the moment you add a dimension and run resample it fails. We are looking into ways to extend the support of other dimensions. We can certainly reuse data.openeo for something but ultimately extend the resample_spatial dim support.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we could do something like this maybe:

    dims = list(data.dims)

    known_dims = [
        data.openeo.band_dims[0],
        data.openeo.temporal_dims[0],
        data.openeo.y_dim,
        data.openeo.x_dim,
    ]

    other_dims = [dim for dim in dims if dim not in known_dims]

    data_cp = data.transpose(*other_dims, *known_dims)

This way we can keep the data.openeo features plus add other dimensions. Of course, some limits and checks would need to be performed on the other dimensions as we cannot realistically allow and support everything.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi Valentina!

I updated the latest commit based on your suggestion to keep the data.openeo functionality. The dummy align is set as well to prevent errors from occurring. Let me know what you think!

Copy link
Collaborator

Choose a reason for hiding this comment

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

LGTM!

The tests are currently failing because you need to pre-commit run --all-files on the branch.

)

#TODO: remove the code below and replace with the code above

# Re-order, this is specifically done for odc reproject
#data_cp = data.transpose(
# data.openeo.band_dims[0],
# data.openeo.temporal_dims[0],
# data.openeo.y_dim,
# data.openeo.x_dim,
#)

if projection is None:
projection = data_cp.rio.crs
Expand Down
Loading