From ab42a05b2daa9887f4015a10c1a9ed13a7a303d4 Mon Sep 17 00:00:00 2001 From: Silvren <237023679+Silvren@users.noreply.github.com> Date: Fri, 17 Jul 2026 16:15:32 +0800 Subject: [PATCH 1/2] Warn about unsupported output format options # Conflicts: # CHANGELOG.md --- CHANGELOG.md | 2 ++ openeo/rest/_testing.py | 10 ++++++-- openeo/rest/connection.py | 30 ++++++++++++++++++++++ openeo/rest/datacube.py | 8 +++++- openeo/rest/vectorcube.py | 6 ++++- tests/rest/datacube/test_datacube100.py | 33 +++++++++++++++++++++++++ tests/rest/datacube/test_vectorcube.py | 17 +++++++++++++ 7 files changed, 102 insertions(+), 4 deletions(-) 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..ebe46c57c 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 _warn_on_invalid_output_format_options( + self, + *, + format: str, + options: Optional[Mapping], + output_formats: Optional[Mapping] = None, + ) -> None: + """Warn when output format options are not advertised by the backend.""" + if not options: + return + + output_formats = output_formats if output_formats is not None else self.list_output_formats() + format_metadata = next( + ( + metadata + for name, metadata in output_formats.items() + if isinstance(name, str) and name.lower() == format.lower() + ), + None, + ) + parameters = format_metadata.get("parameters") if isinstance(format_metadata, Mapping) else None + if 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..5d20f076b 100644 --- a/openeo/rest/datacube.py +++ b/openeo/rest/datacube.py @@ -2403,10 +2403,16 @@ def save_result( of another :py:class:`~openeo.rest.datacube.DataCube` instance. """ if self._connection: - formats = set(self._connection.list_output_formats().keys()) + output_formats = self._connection.list_output_formats() + formats = set(output_formats) # 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._warn_on_invalid_output_format_options( + format=format, + options=options, + output_formats=output_formats, + ) pg = self._build_pgnode( process_id="save_result", diff --git a/openeo/rest/vectorcube.py b/openeo/rest/vectorcube.py index 921cae94e..3dd8b1f79 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: + self._connection._warn_on_invalid_output_format_options(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/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..26cd57105 100644 --- a/tests/rest/datacube/test_vectorcube.py +++ b/tests/rest/datacube/test_vectorcube.py @@ -223,6 +223,23 @@ 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} + + @pytest.mark.parametrize( ["save_result_format", "execute_format", "output_file", "expected"], [ From b68d1a6d8ba53d85099bbd8e1b8f7a1d9feeac1a Mon Sep 17 00:00:00 2001 From: Silvren <237023679+Silvren@users.noreply.github.com> Date: Fri, 17 Jul 2026 16:19:23 +0800 Subject: [PATCH 2/2] Unify output format validation --- openeo/rest/connection.py | 22 +++++++++++----------- openeo/rest/datacube.py | 8 +------- openeo/rest/vectorcube.py | 4 ++-- tests/rest/conftest.py | 10 ++++++++++ tests/rest/datacube/conftest.py | 4 ++++ tests/rest/datacube/test_vectorcube.py | 5 +++++ 6 files changed, 33 insertions(+), 20 deletions(-) diff --git a/openeo/rest/connection.py b/openeo/rest/connection.py index ebe46c57c..037ff52eb 100644 --- a/openeo/rest/connection.py +++ b/openeo/rest/connection.py @@ -856,28 +856,28 @@ def list_input_formats(self) -> dict: def list_output_formats(self) -> dict: return self.list_file_formats().get("output", {}) - def _warn_on_invalid_output_format_options( + def _check_output_format( self, *, format: str, options: Optional[Mapping], - output_formats: Optional[Mapping] = None, ) -> None: - """Warn when output format options are not advertised by the backend.""" - if not options: - return - - output_formats = output_formats if output_formats is not None else self.list_output_formats() - format_metadata = next( + """Validate an output format and warn about options not advertised by the backend.""" + output_formats = self.list_output_formats() + format_name = next( ( - metadata - for name, metadata in output_formats.items() + 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 isinstance(parameters, Mapping): + if options and isinstance(parameters, Mapping): unsupported = sorted(set(options).difference(parameters)) if unsupported: warnings.warn( diff --git a/openeo/rest/datacube.py b/openeo/rest/datacube.py index 5d20f076b..20177356a 100644 --- a/openeo/rest/datacube.py +++ b/openeo/rest/datacube.py @@ -2403,15 +2403,9 @@ def save_result( of another :py:class:`~openeo.rest.datacube.DataCube` instance. """ if self._connection: - output_formats = self._connection.list_output_formats() - formats = set(output_formats) - # 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._warn_on_invalid_output_format_options( + self._connection._check_output_format( format=format, options=options, - output_formats=output_formats, ) pg = self._build_pgnode( diff --git a/openeo/rest/vectorcube.py b/openeo/rest/vectorcube.py index 3dd8b1f79..cbe8b09d7 100644 --- a/openeo/rest/vectorcube.py +++ b/openeo/rest/vectorcube.py @@ -212,8 +212,8 @@ def save_result( of another :py:class:`~openeo.rest.vectorcube.VectorCube` instance. """ format = format or "GeoJSON" - if self._connection: - self._connection._warn_on_invalid_output_format_options(format=format, options=options) + if self._connection and options: + self._connection._check_output_format(format=format, options=options) pg = self._build_pgnode( process_id="save_result", 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_vectorcube.py b/tests/rest/datacube/test_vectorcube.py index 26cd57105..b88f0498e 100644 --- a/tests/rest/datacube/test_vectorcube.py +++ b/tests/rest/datacube/test_vectorcube.py @@ -240,6 +240,11 @@ def test_save_result_format_options_warning(vector_cube, dummy_backend): 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"], [