From 179a852080cba19054d5b3df8cb100add1ea2fda Mon Sep 17 00:00:00 2001 From: Stefaan Lippens Date: Thu, 5 Dec 2024 12:48:42 +0100 Subject: [PATCH] Issue #676 fix usage of Parameter.spatial_extent with load_collection and filter_bbox --- CHANGELOG.md | 1 + openeo/api/process.py | 25 ++++++++++- openeo/rest/datacube.py | 12 +++--- tests/api/test_process.py | 34 ++++++++++++++- tests/rest/datacube/test_datacube100.py | 52 +++++++++++++++++++++-- tests/rest/test_udp.py | 56 +++++++++++++++++++------ 6 files changed, 156 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fe65c0475..cdc2dc486 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - `load_stac`: use fallback temporal dimension when no "cube:dimensions" in STAC Collection ([#666](https://github.com/Open-EO/openeo-python-client/issues/666)) +- Fix usage of `Parameter.spatial_extent()` with `load_collection` and `filter_bbox` ([#676](https://github.com/Open-EO/openeo-python-client/issues/676)) ## [0.35.0] - 2024-11-19 diff --git a/openeo/api/process.py b/openeo/api/process.py index a00f2f9c2..b486d75c0 100644 --- a/openeo/api/process.py +++ b/openeo/api/process.py @@ -410,7 +410,7 @@ def geojson(cls, name: str, description: str = "Geometries specified as GeoJSON @classmethod def temporal_interval( cls, - name: str, + name: str = "temporal_extent", description: str = "Temporal extent specified as two-element array with start and end date/date-time.", **kwargs, ) -> Parameter: @@ -441,3 +441,26 @@ def temporal_interval( }, } return cls(name=name, description=description, schema=schema, **kwargs) + + +def schema_supports(schema: Union[dict, List[dict]], type: str, subtype: Optional[str] = None) -> bool: + """Helper to check if parameter schema supports given type/subtype""" + # TODO: support checking item type in arrays + if isinstance(schema, dict): + actual_type = schema.get("type") + if isinstance(actual_type, str): + if actual_type != type: + return False + elif isinstance(actual_type, list): + if type not in actual_type: + return False + else: + raise ValueError(actual_type) + if subtype: + if schema.get("subtype") != subtype: + return False + return True + elif isinstance(schema, list): + return any(schema_supports(s, type=type, subtype=subtype) for s in schema) + else: + raise ValueError(schema) diff --git a/openeo/rest/datacube.py b/openeo/rest/datacube.py index 3e80edbe5..d5874b727 100644 --- a/openeo/rest/datacube.py +++ b/openeo/rest/datacube.py @@ -25,7 +25,7 @@ import shapely.geometry.base from shapely.geometry import MultiPolygon, Polygon, mapping -from openeo.api.process import Parameter +from openeo.api.process import Parameter, schema_supports from openeo.dates import get_temporal_extent from openeo.internal.documentation import openeo_process from openeo.internal.graph_building import PGNode, ReduceNode, _FromNodeMixin @@ -182,10 +182,10 @@ def load_collection( temporal_extent = cls._get_temporal_extent(extent=temporal_extent) if isinstance(spatial_extent, Parameter): - if spatial_extent.schema.get("type") != "object": + if not schema_supports(spatial_extent.schema, type="object"): warnings.warn( "Unexpected parameterized `spatial_extent` in `load_collection`:" - f" expected schema with type 'object' but got {spatial_extent.schema!r}." + f" expected schema compatible with type 'object' but got {spatial_extent.schema!r}." ) arguments = { 'id': collection_id, @@ -481,7 +481,7 @@ def filter_bbox( crs: Optional[Union[int, str]] = None, base: Optional[float] = None, height: Optional[float] = None, - bbox: Optional[Sequence[float]] = None, + bbox: Union[Sequence[float], Parameter, None] = None, ) -> DataCube: """ Limits the data cube to the specified bounding box. @@ -555,10 +555,10 @@ def filter_bbox( raise ValueError(args) if isinstance(bbox, Parameter): - if bbox.schema.get("type") != "object": + if not schema_supports(bbox.schema, type="object"): warnings.warn( "Unexpected parameterized `extent` in `filter_bbox`:" - f" expected schema with type 'object' but got {bbox.schema!r}." + f" expected schema compatible with type 'object' but got {bbox.schema!r}." ) extent = bbox else: diff --git a/tests/api/test_process.py b/tests/api/test_process.py index 60473abbe..10b866522 100644 --- a/tests/api/test_process.py +++ b/tests/api/test_process.py @@ -1,6 +1,6 @@ import pytest -from openeo.api.process import Parameter +from openeo.api.process import Parameter, schema_supports def test_parameter_defaults(): @@ -261,3 +261,35 @@ def test_parameter_spatial_extent(): }, ], } + + +def test_schema_supports_type_basic(): + schema = {"type": "string"} + assert schema_supports(schema, type="string") is True + assert schema_supports(schema, type="number") is False + + +def test_schema_supports_type_list(): + schema = {"type": ["string", "number"]} + assert schema_supports(schema, type="string") is True + assert schema_supports(schema, type="number") is True + assert schema_supports(schema, type="object") is False + + +def test_schema_supports_subtype(): + schema = {"type": "object", "subtype": "datacube"} + assert schema_supports(schema, type="object") is True + assert schema_supports(schema, type="object", subtype="datacube") is True + assert schema_supports(schema, type="object", subtype="geojson") is False + + +def test_schema_supports_list(): + schema = [ + {"type": "string"}, + {"type": "object", "subtype": "datacube"}, + ] + assert schema_supports(schema, type="string") is True + assert schema_supports(schema, type="number") is False + assert schema_supports(schema, type="object") is True + assert schema_supports(schema, type="object", subtype="datacube") is True + assert schema_supports(schema, type="object", subtype="geojson") is False diff --git a/tests/rest/datacube/test_datacube100.py b/tests/rest/datacube/test_datacube100.py index 052ed78ba..15e057891 100644 --- a/tests/rest/datacube/test_datacube100.py +++ b/tests/rest/datacube/test_datacube100.py @@ -243,7 +243,15 @@ def test_filter_bbox_kwargs(con100: Connection, kwargs, expected): assert node["arguments"]["extent"] == expected -def test_filter_bbox_parameter(con100: Connection): +@pytest.mark.parametrize( + "bbox_param", + [ + Parameter(name="my_bbox", schema={"type": "object"}), + Parameter.spatial_extent(name="my_bbox"), + Parameter.bounding_box(name="my_bbox"), + ], +) +def test_filter_bbox_parameter(con100: Connection, bbox_param): expected = { "process_id": "filter_bbox", "arguments": { @@ -252,7 +260,6 @@ def test_filter_bbox_parameter(con100: Connection): }, "result": True, } - bbox_param = Parameter(name="my_bbox", schema={"type": "object"}) cube = con100.load_collection("S2").filter_bbox(bbox_param) assert _get_leaf_node(cube) == expected @@ -274,14 +281,14 @@ def test_filter_bbox_parameter_invalid_schema(con100: Connection): with pytest.warns( UserWarning, - match="Unexpected parameterized `extent` in `filter_bbox`: expected schema with type 'object' but got {'type': 'string'}.", + match="Unexpected parameterized `extent` in `filter_bbox`: expected schema compatible with type 'object' but got {'type': 'string'}.", ): cube = con100.load_collection("S2").filter_bbox(bbox_param) assert _get_leaf_node(cube) == expected with pytest.warns( UserWarning, - match="Unexpected parameterized `extent` in `filter_bbox`: expected schema with type 'object' but got {'type': 'string'}.", + match="Unexpected parameterized `extent` in `filter_bbox`: expected schema compatible with type 'object' but got {'type': 'string'}.", ): cube = con100.load_collection("S2").filter_bbox(bbox=bbox_param) assert _get_leaf_node(cube) == expected @@ -2301,6 +2308,43 @@ def test_load_collection_parameterized_bands(con100): } +@pytest.mark.parametrize( + ["spatial_extent", "temporal_extent", "spatial_name", "temporal_name"], + [ + ( + Parameter.spatial_extent(), + Parameter.temporal_interval(), + "spatial_extent", + "temporal_extent", + ), + ( + Parameter.bounding_box(name="spatial_extent"), + Parameter.temporal_interval(), + "spatial_extent", + "temporal_extent", + ), + ( + Parameter(name="my_bbox", schema={"type": "object"}), + Parameter(name="dates", schema={"type": "array"}), + "my_bbox", + "dates", + ), + ], +) +def test_load_collection_parameterized_extents(con100, spatial_extent, temporal_extent, spatial_name, temporal_name): + cube = con100.load_collection("S2", spatial_extent=spatial_extent, temporal_extent=temporal_extent) + assert get_download_graph(cube, drop_save_result=True) == { + "loadcollection1": { + "arguments": { + "id": "S2", + "spatial_extent": {"from_parameter": spatial_name}, + "temporal_extent": {"from_parameter": temporal_name}, + }, + "process_id": "load_collection", + }, + } + + def test_apply_dimension_temporal_cumsum_with_target(con100, test_data): cumsum = con100.load_collection("S2").apply_dimension('cumsum', dimension="t", target_dimension="MyNewTime") actual_graph = cumsum.flat_graph() diff --git a/tests/rest/test_udp.py b/tests/rest/test_udp.py index c17743836..8ac23236d 100644 --- a/tests/rest/test_udp.py +++ b/tests/rest/test_udp.py @@ -279,12 +279,28 @@ def test_delete(con100, requests_mock): assert adapter.called -def test_build_parameterized_cube_basic(con100, recwarn): - layer = Parameter.string("layer", description="Collection Id") - dates = Parameter.string("dates", description="Temporal extent") - bbox = Parameter("bbox", schema="object", description="bbox") +@pytest.mark.parametrize( + ["layer", "dates", "bbox"], + [ + ( + Parameter.string("layer", description="Collection Id"), + Parameter.string("dates", description="Temporal extent"), + Parameter("bbox", schema="object", description="bbox"), + ), + ( + Parameter.string("layer", description="Collection Id"), + Parameter.temporal_interval(name="dates"), + Parameter.spatial_extent(name="bbox"), + ), + ( + Parameter.string("layer", description="Collection Id"), + Parameter.temporal_interval(name="dates"), + Parameter.bounding_box(name="bbox"), + ), + ], +) +def test_build_parameterized_cube_filters(con100, layer, dates, bbox, recwarn): cube = con100.load_collection(layer).filter_temporal(dates).filter_bbox(bbox) - assert cube.flat_graph() == { "loadcollection1": { "process_id": "load_collection", @@ -353,12 +369,28 @@ def test_build_parameterized_cube_start_date(con100, recwarn): assert recwarn.list == [] -def test_build_parameterized_cube_load_collection(con100, recwarn): - layer = Parameter.string("layer", description="Collection id") - dates = Parameter.string("dates", description="temporal extent") - bbox = Parameter("bbox", schema="object", description="bbox") +@pytest.mark.parametrize( + ["layer", "dates", "bbox"], + [ + ( + Parameter.string("layer", description="Collection Id"), + Parameter.string("dates", description="Temporal extent"), + Parameter("bbox", schema="object", description="bbox"), + ), + ( + Parameter.string("layer", description="Collection Id"), + Parameter.temporal_interval(name="dates"), + Parameter.spatial_extent(name="bbox"), + ), + ( + Parameter.string("layer", description="Collection Id"), + Parameter.temporal_interval(name="dates"), + Parameter.bounding_box(name="bbox"), + ), + ], +) +def test_build_parameterized_cube_load_collection(con100, recwarn, layer, dates, bbox): cube = con100.load_collection(layer, spatial_extent=bbox, temporal_extent=dates) - assert cube.flat_graph() == { "loadcollection1": { "process_id": "load_collection", @@ -379,7 +411,7 @@ def test_build_parameterized_cube_load_collection_invalid_bbox_schema(con100): bbox = Parameter.string("bbox", description="Spatial extent") with pytest.warns( UserWarning, - match="Unexpected parameterized `spatial_extent` in `load_collection`: expected schema with type 'object' but got {'type': 'string'}.", + match="Unexpected parameterized `spatial_extent` in `load_collection`: expected schema compatible with type 'object' but got {'type': 'string'}.", ): cube = con100.load_collection(layer, spatial_extent=bbox, temporal_extent=dates) @@ -401,7 +433,7 @@ def test_build_parameterized_cube_filter_bbox_invalid_schema(con100): bbox = Parameter.string("bbox", description="Spatial extent") with pytest.warns( UserWarning, - match="Unexpected parameterized `extent` in `filter_bbox`: expected schema with type 'object' but got {'type': 'string'}.", + match="Unexpected parameterized `extent` in `filter_bbox`: expected schema compatible with type 'object' but got {'type': 'string'}.", ): cube = con100.load_collection(layer).filter_bbox(bbox)