diff --git a/CHANGELOG.md b/CHANGELOG.md index 2a5a7cacb..2c70a9e50 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `MultiBackendJobManager`: changed job metadata storage API, to enable working with large databases - `DataCube.apply_polygon()`: rename `polygons` argument to `geometries`, but keep support for legacy `polygons` for now ([#592](https://github.com/Open-EO/openeo-python-client/issues/592), [#511](https://github.com/Open-EO/openeo-processes/issues/511)) +- Disallow ambiguous single string argument in `DataCube.filter_temporal()` ([#628](https://github.com/Open-EO/openeo-python-client/issues/628)) ### Removed diff --git a/openeo/rest/datacube.py b/openeo/rest/datacube.py index b920e7591..d90db5c7e 100644 --- a/openeo/rest/datacube.py +++ b/openeo/rest/datacube.py @@ -318,6 +318,12 @@ def filter_temporal( Arguments ``start_date``, ``end_date`` and ``extent``: add support for year/month shorthand notation as discussed at :ref:`date-shorthand-handling`. """ + if len(args) == 1 and isinstance(args[0], (str)): + raise OpenEoClientException( + f"filter_temporal() with a single string argument ({args[0]!r}) is ambiguous." + f" If you want a half-unbounded interval, use something like filter_temporal({args[0]!r}, None) or use explicit keyword arguments." + f" If you want the full interval covering all of {args[0]!r}, use something like filter_temporal(extent={args[0]!r})." + ) return self.process( process_id='filter_temporal', arguments={ diff --git a/tests/rest/datacube/test_datacube.py b/tests/rest/datacube/test_datacube.py index 9e90db601..33b32dd73 100644 --- a/tests/rest/datacube/test_datacube.py +++ b/tests/rest/datacube/test_datacube.py @@ -4,6 +4,8 @@ - 1.0.0-style DataCube """ + +import contextlib import pathlib from datetime import date, datetime from unittest import mock @@ -14,6 +16,7 @@ import shapely import shapely.geometry +from openeo.api.process import Parameter from openeo.rest import BandMathException, OpenEoClientException from openeo.rest._testing import build_capabilities from openeo.rest.connection import Connection @@ -98,26 +101,39 @@ def test_filter_temporal_basic_extent(s2cube): assert graph['arguments']['extent'] == ["2016-01-01", "2016-03-10"] -@pytest.mark.parametrize("args,kwargs,extent", [ - ((), {}, [None, None]), - (("2016-01-01",), {}, ["2016-01-01", None]), - (("2016-01-01", "2016-03-10"), {}, ["2016-01-01", "2016-03-10"]), - ((date(2016, 1, 1), date(2016, 3, 10)), {}, ["2016-01-01", "2016-03-10"]), - ((datetime(2016, 1, 1, 12, 34), datetime(2016, 3, 10, 23, 45)), {}, - ["2016-01-01T12:34:00Z", "2016-03-10T23:45:00Z"]), - ((), {"start_date": "2016-01-01", "end_date": "2016-03-10"}, ["2016-01-01", "2016-03-10"]), - ((), {"start_date": "2016-01-01"}, ["2016-01-01", None]), - ((), {"end_date": "2016-03-10"}, [None, "2016-03-10"]), - ((), {"start_date": date(2016, 1, 1), "end_date": date(2016, 3, 10)}, ["2016-01-01", "2016-03-10"]), - ((), {"start_date": datetime(2016, 1, 1, 12, 34), "end_date": datetime(2016, 3, 10, 23, 45)}, - ["2016-01-01T12:34:00Z", "2016-03-10T23:45:00Z"]), - ((), {"extent": ("2016-01-01", "2016-03-10")}, ["2016-01-01", "2016-03-10"]), - ((), {"extent": ("2016-01-01", None)}, ["2016-01-01", None]), - ((), {"extent": (None, "2016-03-10")}, [None, "2016-03-10"]), - ((), {"extent": (date(2016, 1, 1), date(2016, 3, 10))}, ["2016-01-01", "2016-03-10"]), - ((), {"extent": (datetime(2016, 1, 1, 12, 34), datetime(2016, 3, 10, 23, 45))}, - ["2016-01-01T12:34:00Z", "2016-03-10T23:45:00Z"]), -]) +@pytest.mark.parametrize( + "args,kwargs,extent", + [ + ((), {}, [None, None]), + (("2016-01-01", "2016-03-10"), {}, ["2016-01-01", "2016-03-10"]), + ((("2016-01-01", "2016-03-10"),), {}, ["2016-01-01", "2016-03-10"]), + ((["2016-01-01", "2016-03-10"],), {}, ["2016-01-01", "2016-03-10"]), + ((date(2016, 1, 1), date(2016, 3, 10)), {}, ["2016-01-01", "2016-03-10"]), + ( + (datetime(2016, 1, 1, 12, 34), datetime(2016, 3, 10, 23, 45)), + {}, + ["2016-01-01T12:34:00Z", "2016-03-10T23:45:00Z"], + ), + ((), {"start_date": "2016-01-01", "end_date": "2016-03-10"}, ["2016-01-01", "2016-03-10"]), + ((), {"start_date": "2016-01-01"}, ["2016-01-01", None]), + ((), {"end_date": "2016-03-10"}, [None, "2016-03-10"]), + ((), {"start_date": date(2016, 1, 1), "end_date": date(2016, 3, 10)}, ["2016-01-01", "2016-03-10"]), + ( + (), + {"start_date": datetime(2016, 1, 1, 12, 34), "end_date": datetime(2016, 3, 10, 23, 45)}, + ["2016-01-01T12:34:00Z", "2016-03-10T23:45:00Z"], + ), + ((), {"extent": ("2016-01-01", "2016-03-10")}, ["2016-01-01", "2016-03-10"]), + ((), {"extent": ("2016-01-01", None)}, ["2016-01-01", None]), + ((), {"extent": (None, "2016-03-10")}, [None, "2016-03-10"]), + ((), {"extent": (date(2016, 1, 1), date(2016, 3, 10))}, ["2016-01-01", "2016-03-10"]), + ( + (), + {"extent": (datetime(2016, 1, 1, 12, 34), datetime(2016, 3, 10, 23, 45))}, + ["2016-01-01T12:34:00Z", "2016-03-10T23:45:00Z"], + ), + ], +) def test_filter_temporal_generic(s2cube, args, kwargs, extent): im = s2cube.filter_temporal(*args, **kwargs) graph = _get_leaf_node(im) @@ -391,6 +407,28 @@ def test_filter_temporal_extent_single_date_string(s2cube: DataCube, api_version assert flat_graph_extent["filtertemporal1"]["arguments"]["extent"] == expected +@pytest.mark.parametrize( + ["arg", "expect_failure"], + [ + ("2024-09-24", True), + ("2024-09", True), + ("2024", True), + (("2024-09-01", "2024-09-10"), False), + (["2024-09-01", "2024-09-10"], False), + (Parameter.temporal_interval("window"), False), + ], +) +def test_filter_temporal_single_arg(s2cube: DataCube, arg, expect_failure): + if expect_failure: + context = pytest.raises( + OpenEoClientException, match="filter_temporal.*with a single string argument.*is ambiguous.*" + ) + else: + context = contextlib.nullcontext() + with context: + _ = s2cube.filter_temporal(arg) + + def test_max_time(s2cube, api_version): im = s2cube.max_time() graph = _get_leaf_node(im, force_flat=True)