From 4a455192962ed36f83b66a7ae89c2c3af9a75fb3 Mon Sep 17 00:00:00 2001 From: Stefaan Lippens Date: Fri, 17 Jan 2025 10:25:34 +0100 Subject: [PATCH] Issue #678/#682 more test coverage of `spatial_extent` handling --- openeo/rest/_testing.py | 2 +- openeo/rest/connection.py | 1 + openeo/rest/datacube.py | 9 +- tests/rest/test_connection.py | 228 +++++++++++++++++++++++++++++++--- tests/rest/test_udp.py | 2 +- 5 files changed, 220 insertions(+), 22 deletions(-) diff --git a/openeo/rest/_testing.py b/openeo/rest/_testing.py index 7940210d6..63e9460c5 100644 --- a/openeo/rest/_testing.py +++ b/openeo/rest/_testing.py @@ -153,7 +153,7 @@ def setup_collection( json={ "id": collection_id, # define temporal and band dim - "cube:dimensions": {"t": {"type": "temporal"}, "bands": {"type": "bands"}}, + "cube:dimensions": cube_dimensions, }, ) return self diff --git a/openeo/rest/connection.py b/openeo/rest/connection.py index 6d087c27b..f687d5ca1 100644 --- a/openeo/rest/connection.py +++ b/openeo/rest/connection.py @@ -1275,6 +1275,7 @@ def load_collection( - a GeoJSON-style dictionary - a path (as :py:class:`str` or :py:class:`~pathlib.Path`) to a local, client-side GeoJSON file, which will be loaded automatically to get the geometries as GeoJSON construct. + - a URL to a publicly accessible GeoJSON document - a :py:class:`~openeo.api.process.Parameter` instance. :param temporal_extent: limit data to specified temporal interval. Typically, just a two-item list or tuple containing start and end date. diff --git a/openeo/rest/datacube.py b/openeo/rest/datacube.py index 7d4d73333..ec3df056b 100644 --- a/openeo/rest/datacube.py +++ b/openeo/rest/datacube.py @@ -164,6 +164,7 @@ def load_collection( - a GeoJSON-style dictionary - a path (as :py:class:`str` or :py:class:`~pathlib.Path`) to a local, client-side GeoJSON file, which will be loaded automatically to get the geometries as GeoJSON construct. + - a URL to a publicly accessible GeoJSON document - a :py:class:`~openeo.api.process.Parameter` instance. :param temporal_extent: limit data to specified temporal interval. Typically, just a two-item list or tuple containing start and end date. @@ -2907,7 +2908,9 @@ def _get_geometry_argument( process_id: str = "n/a", ) -> Union[dict, Parameter, PGNode, _FromNodeMixin, None]: """ - Convert input to a geometry as "geojson" subtype object or vector cube. + Normalize a user input to a openEO-compatible geometry representation, + like a GeoJSON construct, vector cube reference, bounding box construct, + a parameter reference, ... :param crs: value that encodes a coordinate reference system. See :py:func:`openeo.util.normalize_crs` for more details about additional normalization that is applied to this argument. @@ -2919,8 +2922,8 @@ def _get_geometry_argument( if allow_parameter and isinstance(argument, Parameter): if not schema_supports(argument.schema, type="object"): warnings.warn( - f"Unexpected parameterized `{argument_name}` in `{process_id}`:" - f" expected schema compatible with type 'object' but got {argument.schema!r}." + f"Schema mismatch with parameter given to `{argument_name}` in `{process_id}`:" + f" expected a schema compatible with type 'object' but got {argument.schema!r}." ) return argument elif isinstance(argument, _FromNodeMixin): diff --git a/tests/rest/test_connection.py b/tests/rest/test_connection.py index 20c36987a..767b6bfe1 100644 --- a/tests/rest/test_connection.py +++ b/tests/rest/test_connection.py @@ -2396,24 +2396,218 @@ def test_authenticate_oidc_access_token_wrong_provider(self): connection.authenticate_oidc_access_token(access_token="Th3Tok3n!@#", provider_id="nope") -def test_load_collection_arguments_100(requests_mock): - requests_mock.get(API_URL, json={"api_version": "1.0.0"}) - conn = Connection(API_URL) - requests_mock.get(API_URL + "collections/FOO", json={ - "summaries": {"eo:bands": [{"name": "red"}, {"name": "green"}, {"name": "blue"}]} - }) - spatial_extent = {"west": 1, "south": 2, "east": 3, "north": 4} - temporal_extent = ["2019-01-01", "2019-01-22"] - im = conn.load_collection( - "FOO", spatial_extent=spatial_extent, temporal_extent=temporal_extent, bands=["red", "green"] - ) - assert im._pg.process_id == "load_collection" - assert im._pg.arguments == { - "id": "FOO", - "spatial_extent": spatial_extent, - "temporal_extent": temporal_extent, - "bands": ["red", "green"] +class TestLoadCollection: + def test_load_collection_arguments_basic(self, dummy_backend): + spatial_extent = {"west": 1, "south": 2, "east": 3, "north": 4} + temporal_extent = ["2019-01-01", "2019-01-22"] + cube = dummy_backend.connection.load_collection( + "S2", spatial_extent=spatial_extent, temporal_extent=temporal_extent, bands=["B2", "B3"] + ) + cube.execute() + assert dummy_backend.get_sync_pg() == { + "loadcollection1": { + "process_id": "load_collection", + "arguments": { + "id": "S2", + "spatial_extent": {"east": 3, "north": 4, "south": 2, "west": 1}, + "temporal_extent": ["2019-01-01", "2019-01-22"], + "bands": ["B2", "B3"], + }, + "result": True, + } + } + + def test_load_collection_spatial_extent_bbox(self, dummy_backend): + spatial_extent = {"west": 1, "south": 2, "east": 3, "north": 4} + cube = dummy_backend.connection.load_collection("S2", spatial_extent=spatial_extent) + cube.execute() + assert dummy_backend.get_sync_pg()["loadcollection1"]["arguments"] == { + "id": "S2", + "spatial_extent": {"east": 3, "north": 4, "south": 2, "west": 1}, + "temporal_extent": None, + } + + # TODO: make this more reusable + GEOJSON_POINT_01 = {"type": "Point", "coordinates": [3, 52]} + GEOJSON_LINESTRING_01 = {"type": "LineString", "coordinates": [[3, 50], [4, 51], [5, 53]]} + GEOJSON_POLYGON_01 = { + "type": "Polygon", + "coordinates": [[[3, 51], [4, 51], [4, 52], [3, 52], [3, 51]]], + } + GEOJSON_MULTIPOLYGON_01 = { + "type": "MultiPolygon", + "coordinates": [[[[3, 51], [4, 51], [4, 52], [3, 52], [3, 51]]]], + } + GEOJSON_FEATURE_01 = { + "type": "Feature", + "properties": {}, + "geometry": GEOJSON_POLYGON_01, } + GEOJSON_FEATURE_02 = { + "type": "Feature", + "properties": {}, + "geometry": GEOJSON_MULTIPOLYGON_01, + } + GEOJSON_FEATURECOLLECTION_01 = { + "type": "FeatureCollection", + "features": [ + GEOJSON_FEATURE_01, + GEOJSON_FEATURE_02, + ], + } + GEOJSON_GEOMETRYCOLLECTION_01 = { + "type": "GeometryCollection", + "geometries": [ + GEOJSON_POINT_01, + GEOJSON_POLYGON_01, + ], + } + + @pytest.mark.parametrize( + "spatial_extent", + [ + GEOJSON_POLYGON_01, + GEOJSON_MULTIPOLYGON_01, + GEOJSON_FEATURE_01, + GEOJSON_FEATURECOLLECTION_01, + ], + ) + def test_load_collection_spatial_extent_geojson(self, dummy_backend, spatial_extent): + cube = dummy_backend.connection.load_collection("S2", spatial_extent=spatial_extent) + cube.execute() + assert dummy_backend.get_sync_pg()["loadcollection1"]["arguments"] == { + "id": "S2", + "spatial_extent": spatial_extent, + "temporal_extent": None, + } + + @pytest.mark.parametrize( + "spatial_extent", + [GEOJSON_POINT_01, GEOJSON_LINESTRING_01, GEOJSON_GEOMETRYCOLLECTION_01], + ) + def test_load_collection_spatial_extent_geojson_wrong_type(self, con120, spatial_extent): + with pytest.raises(OpenEoClientException, match="Invalid geometry type"): + _ = con120.load_collection("S2", spatial_extent=spatial_extent) + + @pytest.mark.parametrize( + "geojson", + [ + GEOJSON_POLYGON_01, + GEOJSON_MULTIPOLYGON_01, + ], + ) + def test_load_collection_spatial_extent_shapely(self, geojson, dummy_backend): + spatial_extent = shapely.geometry.shape(geojson) + cube = dummy_backend.connection.load_collection("S2", spatial_extent=spatial_extent) + cube.execute() + assert dummy_backend.get_sync_pg()["loadcollection1"]["arguments"] == { + "id": "S2", + "spatial_extent": geojson, + "temporal_extent": None, + } + + @pytest.mark.parametrize( + "geojson", + [ + GEOJSON_POINT_01, + GEOJSON_GEOMETRYCOLLECTION_01, + ], + ) + def test_load_collection_spatial_extent_shapely_wrong_type(self, geojson, dummy_backend): + spatial_extent = shapely.geometry.shape(geojson) + with pytest.raises(OpenEoClientException, match="Invalid geometry type"): + _ = dummy_backend.connection.load_collection("S2", spatial_extent=spatial_extent) + + @pytest.mark.parametrize( + "geojson", + [ + GEOJSON_MULTIPOLYGON_01, + GEOJSON_FEATURECOLLECTION_01, + ], + ) + @pytest.mark.parametrize("path_factory", [str, Path]) + def test_load_collection_spatial_extent_path(self, geojson, dummy_backend, tmp_path, path_factory): + path = tmp_path / "geometry.json" + with path.open("w") as f: + json.dump(geojson, f) + cube = dummy_backend.connection.load_collection("S2", spatial_extent=path_factory(path)) + cube.execute() + assert dummy_backend.get_sync_pg()["loadcollection1"]["arguments"] == { + "id": "S2", + "spatial_extent": geojson, + "temporal_extent": None, + } + + def test_load_collection_spatial_extent_url(self, dummy_backend): + cube = dummy_backend.connection.load_collection("S2", spatial_extent="https://geo.test/geometry.json") + cube.execute() + assert dummy_backend.get_sync_pg() == { + "loadurl1": { + "process_id": "load_url", + "arguments": {"url": "https://geo.test/geometry.json", "format": "GeoJSON"}, + }, + "loadcollection1": { + "process_id": "load_collection", + "arguments": {"id": "S2", "spatial_extent": {"from_node": "loadurl1"}, "temporal_extent": None}, + "result": True, + }, + } + + @pytest.mark.parametrize( + "parameter", + [ + Parameter("zpatial_extent"), + Parameter.spatial_extent("zpatial_extent"), + Parameter.geojson("zpatial_extent"), + ], + ) + def test_load_collection_spatial_extent_parameter(self, dummy_backend, parameter, recwarn): + cube = dummy_backend.connection.load_collection("S2", spatial_extent=parameter) + assert len(recwarn) == 0 + + cube.execute() + assert dummy_backend.get_sync_pg()["loadcollection1"]["arguments"] == { + "id": "S2", + "spatial_extent": {"from_parameter": "zpatial_extent"}, + "temporal_extent": None, + } + + def test_load_collection_spatial_extent_parameter_schema_mismatch(self, dummy_backend, recwarn): + cube = dummy_backend.connection.load_collection( + "S2", spatial_extent=Parameter.number("zpatial_extent", description="foo") + ) + assert [str(w.message) for w in recwarn] == [ + "Schema mismatch with parameter given to `spatial_extent` in `load_collection`: expected a schema compatible with type 'object' but got {'type': 'number'}." + ] + + cube.execute() + assert dummy_backend.get_sync_pg()["loadcollection1"]["arguments"] == { + "id": "S2", + "spatial_extent": {"from_parameter": "zpatial_extent"}, + "temporal_extent": None, + } + + def test_load_collection_spatial_extent_vector_cube(self, dummy_backend): + vector_cube = VectorCube.load_url( + connection=dummy_backend.connection, url="https://geo.test/geometry.json", format="GeoJSON" + ) + cube = dummy_backend.connection.load_collection("S2", spatial_extent=vector_cube) + cube.execute() + assert dummy_backend.get_sync_pg() == { + "loadurl1": { + "process_id": "load_url", + "arguments": {"format": "GeoJSON", "url": "https://geo.test/geometry.json"}, + }, + "loadcollection1": { + "process_id": "load_collection", + "arguments": { + "id": "S2", + "spatial_extent": {"from_node": "loadurl1"}, + "temporal_extent": None, + }, + "result": True, + }, + } def test_load_result(requests_mock): diff --git a/tests/rest/test_udp.py b/tests/rest/test_udp.py index 8ac23236d..cd203a5e7 100644 --- a/tests/rest/test_udp.py +++ b/tests/rest/test_udp.py @@ -411,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 compatible with type 'object' but got {'type': 'string'}.", + match="Schema mismatch with parameter given to `spatial_extent` in `load_collection`: expected a schema compatible with type 'object' but got {'type': 'string'}.", ): cube = con100.load_collection(layer, spatial_extent=bbox, temporal_extent=dates)