Disallow ambiguous single string argument in filter_temporal #628

Open-EO · Sep 24, 2024 · 26bef79 · 26bef79
1 parent 683371f
commit 26bef79
Show file tree

Hide file tree

Showing 3 changed files with 65 additions and 20 deletions.
diff --git 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
@@ -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
@@ -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)