From eff3674c18efc5dfb3a890a9b440126102192c82 Mon Sep 17 00:00:00 2001 From: Jan Van den bosch Date: Fri, 24 Jan 2025 15:15:22 +0100 Subject: [PATCH 1/6] disallow Point Feature https://github.com/Open-EO/openeo-geopyspark-driver/issues/996 --- openeo_driver/ProcessGraphDeserializer.py | 26 ++++++++++++++++++- tests/test_dry_run.py | 31 +++++++++++++++++++++++ tests/test_views_execute.py | 7 ----- 3 files changed, 56 insertions(+), 8 deletions(-) diff --git a/openeo_driver/ProcessGraphDeserializer.py b/openeo_driver/ProcessGraphDeserializer.py index e7ef02a7..a32d008f 100644 --- a/openeo_driver/ProcessGraphDeserializer.py +++ b/openeo_driver/ProcessGraphDeserializer.py @@ -1440,13 +1440,21 @@ def filter_labels(args: ProcessArgs, env: EvalEnv) -> DriverDataCube: def _extract_bbox_extent(args: dict, field="extent", process_id="filter_bbox", handle_geojson=False) -> dict: extent = extract_arg(args, name=field, process_id=process_id) + # TODO: handle vector cube if handle_geojson and extent.get("type") in [ "Polygon", "MultiPolygon", - "GeometryCollection", + "GeometryCollection", # TODO: disallow GeometryCollection? "Feature", "FeatureCollection", ]: + if not _contains_polygon(extent): + raise ProcessParameterInvalidException( + parameter=field, + process=process_id, + reason="unsupported GeoJSON; requires at least one Polygon or MultiPolygon", + ) + w, s, e, n = DriverVectorCube.from_geojson(extent).get_bounding_box() # TODO: support (non-standard) CRS field in GeoJSON? d = {"west": w, "south": s, "east": e, "north": n, "crs": "EPSG:4326"} @@ -1462,6 +1470,22 @@ def _extract_bbox_extent(args: dict, field="extent", process_id="filter_bbox", h return d +def _contains_polygon(geojson: dict) -> bool: + if geojson["type"] in ["Polygon", "MultiPolygon"]: + return True + + if geojson["type"] == "Feature": + return _contains_polygon(geojson["geometry"]) + + if geojson["type"] == "FeatureCollection": + return any(_contains_polygon(feature) for feature in geojson["features"]) + + if geojson["type"] == "GeometryCollection": + return any(_contains_polygon(geometry) for geometry in geojson["geometries"]) + + return False + + @process def filter_bbox(args: ProcessArgs, env: EvalEnv) -> DriverDataCube: cube: DriverDataCube = args.get_required("data", expected_type=DriverDataCube) diff --git a/tests/test_dry_run.py b/tests/test_dry_run.py index 79507d1e..52ba1138 100644 --- a/tests/test_dry_run.py +++ b/tests/test_dry_run.py @@ -1447,6 +1447,37 @@ def test_load_stac_properties(dry_run_env, dry_run_tracer): ] +def test_load_stac_spatial_extent_requires_a_polygon(dry_run_tracer, backend_implementation): + pg = { + "loadstac1": { + "process_id": "load_stac", + "arguments": { + "url": "https://stac.test", + "spatial_extent": { + "type": "Feature", + "geometry": { + "type": "Point", + "coordinates": [5.0, 50.0] + } + } + }, + "result": True, + } + } + + dry_run_env = EvalEnv( + {ENV_DRY_RUN_TRACER: dry_run_tracer, "backend_implementation": backend_implementation, "version": "2.0.0"} + ) + + with pytest.raises(OpenEOApiException) as e: + evaluate(pg, dry_run_env) + + assert e.value.message == ( + "The value passed for parameter 'spatial_extent' in process 'load_stac' is invalid: " + "unsupported GeoJSON; requires at least one Polygon or MultiPolygon" + ) + + @pytest.mark.parametrize( ["arguments", "expected"], [ diff --git a/tests/test_views_execute.py b/tests/test_views_execute.py index ae44d661..b9a5ae46 100644 --- a/tests/test_views_execute.py +++ b/tests/test_views_execute.py @@ -352,13 +352,6 @@ def test_load_collection_filter(api): ), {"west": 11, "south": 22, "east": 44, "north": 55, "crs": "EPSG:4326"}, ), - ( - as_geojson_feature_collection( - shapely.geometry.Point(2, 3), - shapely.geometry.Point(4, 5), - ), - {"west": 2, "south": 3, "east": 4, "north": 5, "crs": "EPSG:4326"}, - ), ], ) def test_load_collection_spatial_extent_geojson(api, spatial_extent, expected): From 4e3f2894f43795ff41d8ba13fd3d771b247cd679 Mon Sep 17 00:00:00 2001 From: Jan Van den bosch Date: Fri, 24 Jan 2025 15:23:48 +0100 Subject: [PATCH 2/6] test FeatureCollection of Points https://github.com/Open-EO/openeo-geopyspark-driver/issues/996 --- tests/test_dry_run.py | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/tests/test_dry_run.py b/tests/test_dry_run.py index 52ba1138..47609595 100644 --- a/tests/test_dry_run.py +++ b/tests/test_dry_run.py @@ -1447,19 +1447,28 @@ def test_load_stac_properties(dry_run_env, dry_run_tracer): ] -def test_load_stac_spatial_extent_requires_a_polygon(dry_run_tracer, backend_implementation): +@pytest.mark.parametrize( + "spatial_extent", + [ + {"type": "Feature", "geometry": {"type": "Point", "coordinates": [5, 50]}}, + { + "type": "FeatureCollection", + "features": [ + { + "type": "Feature", + "geometry": {"type": "Feature", "geometry": {"type": "Point", "coordinates": [5, 50]}}, + } + ], + }, + ], +) +def test_load_stac_spatial_extent_requires_a_polygon(dry_run_tracer, backend_implementation, spatial_extent): pg = { "loadstac1": { "process_id": "load_stac", "arguments": { "url": "https://stac.test", - "spatial_extent": { - "type": "Feature", - "geometry": { - "type": "Point", - "coordinates": [5.0, 50.0] - } - } + "spatial_extent": spatial_extent, }, "result": True, } From 08d1337e0f5ac5a7efdec0479f92808bab582257 Mon Sep 17 00:00:00 2001 From: Jan Van den bosch Date: Fri, 24 Jan 2025 16:05:46 +0100 Subject: [PATCH 3/6] test some supported geometries https://github.com/Open-EO/openeo-geopyspark-driver/issues/996 --- tests/test_dry_run.py | 57 ++++++++++++++++++++++++++++--------------- 1 file changed, 38 insertions(+), 19 deletions(-) diff --git a/tests/test_dry_run.py b/tests/test_dry_run.py index 47609595..2f7c7a49 100644 --- a/tests/test_dry_run.py +++ b/tests/test_dry_run.py @@ -1,3 +1,4 @@ +from contextlib import nullcontext from pathlib import Path from unittest import mock @@ -18,7 +19,7 @@ ProcessType, ) from openeo_driver.dummy.dummy_backend import DummyVectorCube -from openeo_driver.errors import OpenEOApiException +from openeo_driver.errors import OpenEOApiException, ProcessParameterInvalidException from openeo_driver.ProcessGraphDeserializer import ( ENV_DRY_RUN_TRACER, ENV_MAX_BUFFER, @@ -1448,21 +1449,44 @@ def test_load_stac_properties(dry_run_env, dry_run_tracer): @pytest.mark.parametrize( - "spatial_extent", + ["spatial_extent", "expectation"], [ - {"type": "Feature", "geometry": {"type": "Point", "coordinates": [5, 50]}}, - { - "type": "FeatureCollection", - "features": [ - { - "type": "Feature", - "geometry": {"type": "Feature", "geometry": {"type": "Point", "coordinates": [5, 50]}}, - } - ], - }, + ({"type": "Polygon", "coordinates": [[[0, 0], [1, 1], [1, 0]]]}, nullcontext()), + ( + { + "type": "Feature", + "geometry": {"type": "MultiPolygon", "coordinates": [[[[0, 0], [1, 1], [1, 0]]]]}, + "properties": {}, + }, + nullcontext(), + ), + ( + {"type": "Feature", "geometry": {"type": "Point", "coordinates": [5, 50]}}, + pytest.raises( + ProcessParameterInvalidException, + match=r"unsupported GeoJSON; requires at least one Polygon or MultiPolygon$", + ), + ), + ( + { + "type": "FeatureCollection", + "features": [ + { + "type": "Feature", + "geometry": {"type": "Point", "coordinates": [5, 50]}, + } + ], + }, + pytest.raises( + ProcessParameterInvalidException, + match=r"unsupported GeoJSON; requires at least one Polygon or MultiPolygon$", + ), + ), ], ) -def test_load_stac_spatial_extent_requires_a_polygon(dry_run_tracer, backend_implementation, spatial_extent): +def test_load_stac_spatial_extent_requires_a_polygon( + dry_run_tracer, backend_implementation, spatial_extent, expectation +): pg = { "loadstac1": { "process_id": "load_stac", @@ -1478,14 +1502,9 @@ def test_load_stac_spatial_extent_requires_a_polygon(dry_run_tracer, backend_imp {ENV_DRY_RUN_TRACER: dry_run_tracer, "backend_implementation": backend_implementation, "version": "2.0.0"} ) - with pytest.raises(OpenEOApiException) as e: + with expectation: evaluate(pg, dry_run_env) - assert e.value.message == ( - "The value passed for parameter 'spatial_extent' in process 'load_stac' is invalid: " - "unsupported GeoJSON; requires at least one Polygon or MultiPolygon" - ) - @pytest.mark.parametrize( ["arguments", "expected"], From 86ee6672916d731d7a8d750cf81df71ab97f9a8d Mon Sep 17 00:00:00 2001 From: Jan Van den bosch Date: Thu, 6 Feb 2025 07:29:11 +0100 Subject: [PATCH 4/6] tune TODOs https://github.com/Open-EO/openeo-geopyspark-driver/issues/996 --- openeo_driver/ProcessGraphDeserializer.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/openeo_driver/ProcessGraphDeserializer.py b/openeo_driver/ProcessGraphDeserializer.py index a32d008f..37852017 100644 --- a/openeo_driver/ProcessGraphDeserializer.py +++ b/openeo_driver/ProcessGraphDeserializer.py @@ -1440,11 +1440,12 @@ def filter_labels(args: ProcessArgs, env: EvalEnv) -> DriverDataCube: def _extract_bbox_extent(args: dict, field="extent", process_id="filter_bbox", handle_geojson=False) -> dict: extent = extract_arg(args, name=field, process_id=process_id) - # TODO: handle vector cube + # TODO #114: support vector cube if handle_geojson and extent.get("type") in [ "Polygon", "MultiPolygon", - "GeometryCollection", # TODO: disallow GeometryCollection? + "GeometryCollection", # TODO #71 #114: deprecate GeometryCollection + "GeometryCollection", "Feature", "FeatureCollection", ]: From 0064a03607b522c4b77694a03e48ab8a29698083 Mon Sep 17 00:00:00 2001 From: Jan Van den bosch Date: Thu, 6 Feb 2025 08:37:21 +0100 Subject: [PATCH 5/6] update version and CHANGELOG https://github.com/Open-EO/openeo-geopyspark-driver/issues/996 --- CHANGELOG.md | 2 ++ openeo_driver/_version.py | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7ce4a817..0a2a5ec8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,8 +21,10 @@ and start a new "In Progress" section above it. ## In progress +## 0.124.0 - Better argument validation in `resample_spatial`/`resample_cube_spatial` (related to [Open-EO/openeo-python-client#690](https://github.com/Open-EO/openeo-python-client/issues/690)) - Improve `resample_spatial`/`resample_cube_spatial` metadata tracking in dry-run ([#348](https://github.com/Open-EO/openeo-python-driver/issues/348)) +- `load_collection`/`load_stac`: `spatial_extent` requires a (Multi)Polygon geometry ([Open-EO/openeo-geopyspark-driver#996](https://github.com/Open-EO/openeo-geopyspark-driver/issues/996)) ## 0.123.0 diff --git a/openeo_driver/_version.py b/openeo_driver/_version.py index 573fb798..de09e767 100644 --- a/openeo_driver/_version.py +++ b/openeo_driver/_version.py @@ -1 +1 @@ -__version__ = "0.123.0a1" +__version__ = "0.124.0a1" From 0c1743d01d662692f4d9f080f39cd04c5daf2507 Mon Sep 17 00:00:00 2001 From: Jan Van den bosch Date: Mon, 10 Feb 2025 15:59:39 +0100 Subject: [PATCH 6/6] remove duplicate entry https://github.com/Open-EO/openeo-geopyspark-driver/issues/996 Co-authored-by: Jeroen Dries --- openeo_driver/ProcessGraphDeserializer.py | 1 - 1 file changed, 1 deletion(-) diff --git a/openeo_driver/ProcessGraphDeserializer.py b/openeo_driver/ProcessGraphDeserializer.py index f9c93dd7..0ccad342 100644 --- a/openeo_driver/ProcessGraphDeserializer.py +++ b/openeo_driver/ProcessGraphDeserializer.py @@ -1465,7 +1465,6 @@ def _extract_bbox_extent(args: dict, field="extent", process_id="filter_bbox", h "Polygon", "MultiPolygon", "GeometryCollection", # TODO #71 #114: deprecate GeometryCollection - "GeometryCollection", "Feature", "FeatureCollection", ]: