diff --git a/CHANGELOG.md b/CHANGELOG.md index 8037ed845..496aab5f5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- Warn when `save_result` uses output format options that are not advertised by the backend. ([#649](https://github.com/Open-EO/openeo-python-client/issues/649)) + ### Changed ### Removed diff --git a/openeo/rest/_testing.py b/openeo/rest/_testing.py index 6e8423c26..d4de1b97e 100644 --- a/openeo/rest/_testing.py +++ b/openeo/rest/_testing.py @@ -181,11 +181,17 @@ def setup_collection( ) return self - def setup_file_format(self, name: str, type: str = "output", gis_data_types: Iterable[str] = ("raster",)): + def setup_file_format( + self, + name: str, + type: str = "output", + gis_data_types: Iterable[str] = ("raster",), + parameters: Optional[dict] = None, + ): self.file_formats[type][name] = { "title": name, "gis_data_types": list(gis_data_types), - "parameters": {}, + "parameters": parameters or {}, } self._requests_mock.get(self.connection.build_url("/file_formats"), json=self.file_formats) return self diff --git a/openeo/rest/connection.py b/openeo/rest/connection.py index 6a34f922a..037ff52eb 100644 --- a/openeo/rest/connection.py +++ b/openeo/rest/connection.py @@ -856,6 +856,36 @@ def list_input_formats(self) -> dict: def list_output_formats(self) -> dict: return self.list_file_formats().get("output", {}) + def _check_output_format( + self, + *, + format: str, + options: Optional[Mapping], + ) -> None: + """Validate an output format and warn about options not advertised by the backend.""" + output_formats = self.list_output_formats() + format_name = next( + ( + name + for name in output_formats + if isinstance(name, str) and name.lower() == format.lower() + ), + None, + ) + if format_name is None: + raise ValueError(f"Invalid format {format!r}. Should be one of {set(output_formats)}") + + format_metadata = output_formats[format_name] + parameters = format_metadata.get("parameters") if isinstance(format_metadata, Mapping) else None + if options and isinstance(parameters, Mapping): + unsupported = sorted(set(options).difference(parameters)) + if unsupported: + warnings.warn( + f"Unsupported options {unsupported} for output format {format!r} " + f"(supported options: {sorted(parameters)}).", + stacklevel=3, + ) + list_file_types = legacy_alias( list_output_formats, "list_file_types", since="0.4.6" ) diff --git a/openeo/rest/datacube.py b/openeo/rest/datacube.py index 8df992621..20177356a 100644 --- a/openeo/rest/datacube.py +++ b/openeo/rest/datacube.py @@ -2403,10 +2403,10 @@ def save_result( of another :py:class:`~openeo.rest.datacube.DataCube` instance. """ if self._connection: - formats = set(self._connection.list_output_formats().keys()) - # TODO: map format to correct casing too? - if format.lower() not in {f.lower() for f in formats}: - raise ValueError("Invalid format {f!r}. Should be one of {s}".format(f=format, s=formats)) + self._connection._check_output_format( + format=format, + options=options, + ) pg = self._build_pgnode( process_id="save_result", diff --git a/openeo/rest/vectorcube.py b/openeo/rest/vectorcube.py index 921cae94e..cbe8b09d7 100644 --- a/openeo/rest/vectorcube.py +++ b/openeo/rest/vectorcube.py @@ -211,11 +211,15 @@ def save_result( returns a :py:class:`~openeo.rest.result.SaveResult` instance instead of another :py:class:`~openeo.rest.vectorcube.VectorCube` instance. """ + format = format or "GeoJSON" + if self._connection and options: + self._connection._check_output_format(format=format, options=options) + pg = self._build_pgnode( process_id="save_result", arguments={ "data": self, - "format": format or "GeoJSON", + "format": format, # TODO: leave out options if unset? "options": options or {}, }, diff --git a/tests/rest/conftest.py b/tests/rest/conftest.py index 7979914a1..946a8a6a2 100644 --- a/tests/rest/conftest.py +++ b/tests/rest/conftest.py @@ -88,6 +88,11 @@ def dummy_backend(requests_mock, con120) -> DummyBackend: dummy_backend.setup_collection("S2") dummy_backend.setup_file_format("GTiff") dummy_backend.setup_file_format("netCDF") + dummy_backend.setup_file_format( + "GeoJSON", + gis_data_types=("vector",), + parameters={"precision": {"type": "integer"}}, + ) return dummy_backend @@ -100,6 +105,11 @@ def another_dummy_backend(requests_mock) -> DummyBackend: another_dummy_backend.setup_collection("S2") another_dummy_backend.setup_file_format("GTiff") another_dummy_backend.setup_file_format("netCDF") + another_dummy_backend.setup_file_format( + "GeoJSON", + gis_data_types=("vector",), + parameters={"precision": {"type": "integer"}}, + ) return another_dummy_backend diff --git a/tests/rest/datacube/conftest.py b/tests/rest/datacube/conftest.py index ddf0c1170..6da53cd2d 100644 --- a/tests/rest/datacube/conftest.py +++ b/tests/rest/datacube/conftest.py @@ -48,6 +48,10 @@ def _setup_connection(api_version, requests_mock, build_capabilities_kwargs: Opt "GTiff": {"gis_data_types": ["raster"]}, "netCDF": {"gis_data_types": ["raster"]}, "csv": {"gis_data_types": ["table"]}, + "GeoJSON": { + "gis_data_types": ["vector"], + "parameters": {"precision": {"type": "integer"}}, + }, } }, ) diff --git a/tests/rest/datacube/test_datacube100.py b/tests/rest/datacube/test_datacube100.py index 40f7ef10f..cc76c135b 100644 --- a/tests/rest/datacube/test_datacube100.py +++ b/tests/rest/datacube/test_datacube100.py @@ -2965,6 +2965,39 @@ def test_save_result_format(con100, requests_mock): cube.save_result(format="pNg") +def test_save_result_format_options_warning(con100, requests_mock): + requests_mock.get( + API_URL + "/file_formats", + json={ + "output": { + "GTiff": { + "gis_data_types": ["raster"], + "parameters": { + "compression": {"type": "string"}, + "tile_size": {"type": "integer"}, + }, + } + } + }, + ) + + cube = con100.load_collection("S2") + with pytest.warns( + UserWarning, + match=r"Unsupported options \['overview_level'\] for output format 'gtiff' " + r"\(supported options: \['compression', 'tile_size'\]\)\.", + ): + result = cube.save_result( + format="gtiff", + options={"compression": "DEFLATE", "overview_level": 4}, + ) + + assert result.flat_graph()["saveresult1"]["arguments"]["options"] == { + "compression": "DEFLATE", + "overview_level": 4, + } + + EXPECTED_JSON_EXPORT_S2_NDVI = textwrap.dedent( """\ { diff --git a/tests/rest/datacube/test_vectorcube.py b/tests/rest/datacube/test_vectorcube.py index 53cc0afea..b88f0498e 100644 --- a/tests/rest/datacube/test_vectorcube.py +++ b/tests/rest/datacube/test_vectorcube.py @@ -223,6 +223,28 @@ def test_save_result_and_download_filename( assert output_path.read_bytes() == DummyBackend.DEFAULT_RESULT +def test_save_result_format_options_warning(vector_cube, dummy_backend): + dummy_backend.setup_file_format( + "GeoJSON", + gis_data_types=("vector",), + parameters={"indent": {"type": "integer"}}, + ) + + with pytest.warns( + UserWarning, + match=r"Unsupported options \['invalid'\] for output format 'geojson' " + r"\(supported options: \['indent'\]\)\.", + ): + result = vector_cube.save_result(format="geojson", options={"indent": 2, "invalid": True}) + + assert result.flat_graph()["saveresult1"]["arguments"]["options"] == {"indent": 2, "invalid": True} + + +def test_save_result_invalid_format(vector_cube): + with pytest.raises(ValueError, match="Invalid format 'invalid'"): + vector_cube.save_result(format="invalid", options={"indent": 2}) + + @pytest.mark.parametrize( ["save_result_format", "execute_format", "output_file", "expected"], [