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

resample(_cube)_spatial: metadata transform in dry run #348

Open
jdries opened this issue Dec 19, 2024 · 3 comments
Open

resample(_cube)_spatial: metadata transform in dry run #348

jdries opened this issue Dec 19, 2024 · 3 comments
Assignees

Comments

@jdries
Copy link
Contributor

jdries commented Dec 19, 2024

dry run requires accurate info on cube metadata (crs, resolution, extent) to make decisions on global extent and other load parameters (resolution, target crs)

currently, this is inaccurate because if a target cube has been resampled, dry run will not 'see' this, can lead to bugs so quite important

@soxofaan
Copy link
Member

soxofaan commented Jan 21, 2025

resample_spatial (and resample_cube_spatial) arguments are already being tracked during dry run:

def resample_spatial(
self,
resolution: Union[float, Tuple[float, float]],
projection: Union[int, str] = None,
method: str = "near",
align: str = "upper-left",
):
return self._process(
"resample_spatial",
arguments={"resolution": resolution, "projection": projection, "method": method, "align": align}
)

They are being included in source constraints:

args = resampling_op.get_arguments_by_operation("resample_spatial")
if args:
resolution = args[0]["resolution"]
if not isinstance(resolution,list):
resolution = [resolution,resolution]
projection = args[0]["projection"]
method = args[0].get("method","near")
constraints["resample"] = {"target_crs": projection, "resolution": resolution, "method": method}

extracted to LoadParameters():

params.target_crs = constraints.get("resample", {}).get("target_crs",None)
params.target_resolution = constraints.get("resample", {}).get("resolution", None)
params.resample_method = constraints.get("resample", {}).get("method", "near")

and used in load_collection from geopyspark driver:
https://github.com/Open-EO/openeo-geopyspark-driver/blob/f5a1532634336f72dd56b9e44e4c75365ad2ec2b/openeogeotrellis/layercatalog.py#L238-L251

So I'm not sure it's actually necessary to add something new, or an existing (broken?) feature just has to be fixed

@soxofaan
Copy link
Member

Just finished/merged Open-EO/openeo-python-client#690 with Open-EO/openeo-python-client#712
which hopefully automatically improves the situation in the python driver.
I'll set up a unit test to verify

@soxofaan
Copy link
Member

improved resample resolution/projection handling in dry run with 7116e40

(added a test about resample_cube_spatial that gets resolution from other resample_spatialed cube)

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

No branches or pull requests

2 participants