Issue #509 document Parameter support for load_collection and filter_bbox

also warn when parameter schema is invalid

Author: soxofaan

openeo/rest/connection.py

@@ -1140,7 +1140,7 @@ def datacube_from_json(self, src: Union[str, Path], parameters: Optional[dict] =
     def load_collection(
         self,
         collection_id: Union[str, Parameter],
-        spatial_extent: Optional[Dict[str, float]] = None,
+        spatial_extent: Union[Dict[str, float], Parameter, None] = None,
         temporal_extent: Union[Sequence[InputDate], Parameter, str, None] = None,
         bands: Union[None, List[str], Parameter] = None,
         properties: Union[

openeo/rest/datacube.py

@@ -121,7 +121,7 @@ def load_collection(
         cls,
         collection_id: Union[str, Parameter],
         connection: Connection = None,
-        spatial_extent: Optional[Dict[str, float]] = None,
+        spatial_extent: Union[Dict[str, float], Parameter, None] = None,
         temporal_extent: Union[Sequence[InputDate], Parameter, str, None] = None,
         bands: Union[None, List[str], Parameter] = None,
         fetch_metadata: bool = True,
@@ -157,6 +157,13 @@ def load_collection(
         """
         if temporal_extent:
             temporal_extent = cls._get_temporal_extent(extent=temporal_extent)
+
+        if isinstance(spatial_extent, Parameter):
+            if spatial_extent.schema.get("type") != "object":
+                warnings.warn(
+                    "Unexpected parameterized `spatial_extent` in `load_collection`:"
+                    f" expected schema with type 'object' but got {spatial_extent.schema!r}."
+                )
         arguments = {
             'id': collection_id,
             # TODO: spatial_extent could also be a "geojson" subtype object, so we might want to allow (and convert) shapely shapes as well here.
@@ -250,11 +257,13 @@ def _get_temporal_extent(
             return args[0]
         elif len(args) == 0 and isinstance(extent, Parameter):
             assert start_date is None and end_date is None
+            # TODO: warn about unexpected parameter schema
             return extent
         else:
             def convertor(d: Any) -> Any:
                 # TODO: can this be generalized through _FromNodeMixin?
                 if isinstance(d, Parameter) or isinstance(d, PGNode):
+                    # TODO: warn about unexpected parameter schema
                     return d
                 elif isinstance(d, ProcessBuilderBase):
                     return d.pgnode
@@ -385,6 +394,11 @@ def filter_bbox(
                 raise ValueError(args)
 
         if isinstance(bbox, Parameter):
+            if bbox.schema.get("type") != "object":
+                warnings.warn(
+                    "Unexpected parameterized `extent` in `filter_bbox`:"
+                    f" expected schema with type 'object' but got {bbox.schema!r}."
+                )
             extent = bbox
         else:
             if bbox:

tests/rest/datacube/test_datacube100.py

@@ -267,6 +267,32 @@ def test_filter_bbox_parameter(con100: Connection):
     assert _get_leaf_node(cube) == expected
 
 
+def test_filter_bbox_parameter_invalid_schema(con100: Connection):
+    expected = {
+        "process_id": "filter_bbox",
+        "arguments": {
+            "data": {"from_node": "loadcollection1"},
+            "extent": {"from_parameter": "my_bbox"},
+        },
+        "result": True,
+    }
+    bbox_param = Parameter(name="my_bbox", schema={"type": "string"})
+
+    with pytest.warns(
+        UserWarning,
+        match="Unexpected parameterized `extent` in `filter_bbox`: expected schema with type 'object' but got {'type': 'string'}.",
+    ):
+        cube = con100.load_collection("S2").filter_bbox(bbox_param)
+    assert _get_leaf_node(cube) == expected
+
+    with pytest.warns(
+        UserWarning,
+        match="Unexpected parameterized `extent` in `filter_bbox`: expected schema with type 'object' but got {'type': 'string'}.",
+    ):
+        cube = con100.load_collection("S2").filter_bbox(bbox=bbox_param)
+    assert _get_leaf_node(cube) == expected
+
+
 @pytest.mark.parametrize(["args", "expected"], [
     ((3, 4, 52, 51,), {"west": 3, "south": 51, "east": 4, "north": 52}),
     ((3, 4, 52, 51, 4326,), {"west": 3, "south": 51, "east": 4, "north": 52, "crs": 4326}),

tests/rest/test_udp.py

@@ -244,10 +244,10 @@ def test_delete(con100, requests_mock):
     assert adapter.called
 
 
-def test_build_parameterized_cube_basic(con100):
-    layer = Parameter.string("layer")
-    dates = Parameter.string("dates")
-    bbox = Parameter("bbox", schema="object")
+def test_build_parameterized_cube_basic(con100, recwarn):
+    layer = Parameter.string("layer", description="Collection Id")
+    dates = Parameter.string("dates", description="Temporal extent")
+    bbox = Parameter("bbox", schema="object", description="bbox")
     cube = con100.load_collection(layer).filter_temporal(dates).filter_bbox(bbox)
 
     assert cube.flat_graph() == {
@@ -265,6 +265,7 @@ def test_build_parameterized_cube_basic(con100):
             "result": True,
         }
     }
+    assert recwarn.list == []
 
 
 def test_build_parameterized_cube_single_date(con100):
@@ -293,10 +294,10 @@ def test_build_parameterized_cube_single_date(con100):
     }
 
 
-def test_build_parameterized_cube_start_date(con100):
-    layer = Parameter.string("layer")
-    start = Parameter.string("start")
-    bbox = Parameter("bbox", schema="object")
+def test_build_parameterized_cube_start_date(con100, recwarn):
+    layer = Parameter.string("layer", description="Collection id")
+    start = Parameter.string("start", description="Start date")
+    bbox = Parameter("bbox", schema="object", description="Bbox")
     cube = con100.load_collection(layer).filter_temporal(start, None).filter_bbox(bbox)
 
     assert cube.flat_graph() == {
@@ -314,12 +315,13 @@ def test_build_parameterized_cube_start_date(con100):
             "result": True,
         }
     }
+    assert recwarn.list == []
 
 
-def test_build_parameterized_cube_load_collection(con100):
-    layer = Parameter.string("layer")
-    dates = Parameter.string("dates")
-    bbox = Parameter("bbox", schema="object")
+def test_build_parameterized_cube_load_collection(con100, recwarn):
+    layer = Parameter.string("layer", description="Collection id")
+    dates = Parameter.string("dates", description="temporal extent")
+    bbox = Parameter("bbox", schema="object", description="bbox")
     cube = con100.load_collection(layer, spatial_extent=bbox, temporal_extent=dates)
 
     assert cube.flat_graph() == {
@@ -333,6 +335,52 @@ def test_build_parameterized_cube_load_collection(con100):
             "result": True,
         }
     }
+    assert recwarn.list == []
+
+
+def test_build_parameterized_cube_load_collection_invalid_bbox_schema(con100):
+    layer = Parameter.string("layer", description="Collection id")
+    dates = Parameter.string("dates", description="Temporal extent")
+    bbox = Parameter.string("bbox", description="Spatial extent")
+    with pytest.warns(
+        UserWarning,
+        match="Unexpected parameterized `spatial_extent` in `load_collection`: expected schema with type 'object' but got {'type': 'string'}.",
+    ):
+        cube = con100.load_collection(layer, spatial_extent=bbox, temporal_extent=dates)
+
+    assert cube.flat_graph() == {
+        "loadcollection1": {
+            "process_id": "load_collection",
+            "arguments": {
+                "id": {"from_parameter": "layer"},
+                "temporal_extent": {"from_parameter": "dates"},
+                "spatial_extent": {"from_parameter": "bbox"},
+            },
+            "result": True,
+        }
+    }
+
+
+def test_build_parameterized_cube_filter_bbox_invalid_schema(con100):
+    layer = Parameter.string("layer", description="Collection id")
+    bbox = Parameter.string("bbox", description="Spatial extent")
+    with pytest.warns(
+        UserWarning,
+        match="Unexpected parameterized `extent` in `filter_bbox`: expected schema with type 'object' but got {'type': 'string'}.",
+    ):
+        cube = con100.load_collection(layer).filter_bbox(bbox)
+
+    assert cube.flat_graph() == {
+        "loadcollection1": {
+            "process_id": "load_collection",
+            "arguments": {"id": {"from_parameter": "layer"}, "temporal_extent": None, "spatial_extent": None},
+        },
+        "filterbbox1": {
+            "process_id": "filter_bbox",
+            "arguments": {"data": {"from_node": "loadcollection1"}, "extent": {"from_parameter": "bbox"}},
+            "result": True,
+        },
+    }
 
 
 def test_build_parameterized_cube_load_collection_band(con100):