Skip to content

Commit

Permalink
Issue #676 fix usage of Parameter.spatial_extent with load_collection
Browse files Browse the repository at this point in the history
and filter_bbox
  • Loading branch information
soxofaan committed Dec 5, 2024
1 parent 3fd041a commit 179a852
Show file tree
Hide file tree
Showing 6 changed files with 156 additions and 24 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Fixed

- `load_stac`: use fallback temporal dimension when no "cube:dimensions" in STAC Collection ([#666](https://github.com/Open-EO/openeo-python-client/issues/666))
- Fix usage of `Parameter.spatial_extent()` with `load_collection` and `filter_bbox` ([#676](https://github.com/Open-EO/openeo-python-client/issues/676))

## [0.35.0] - 2024-11-19

Expand Down
25 changes: 24 additions & 1 deletion openeo/api/process.py
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,7 @@ def geojson(cls, name: str, description: str = "Geometries specified as GeoJSON
@classmethod
def temporal_interval(
cls,
name: str,
name: str = "temporal_extent",
description: str = "Temporal extent specified as two-element array with start and end date/date-time.",
**kwargs,
) -> Parameter:
Expand Down Expand Up @@ -441,3 +441,26 @@ def temporal_interval(
},
}
return cls(name=name, description=description, schema=schema, **kwargs)


def schema_supports(schema: Union[dict, List[dict]], type: str, subtype: Optional[str] = None) -> bool:
"""Helper to check if parameter schema supports given type/subtype"""
# TODO: support checking item type in arrays
if isinstance(schema, dict):
actual_type = schema.get("type")
if isinstance(actual_type, str):
if actual_type != type:
return False
elif isinstance(actual_type, list):
if type not in actual_type:
return False
else:
raise ValueError(actual_type)
if subtype:
if schema.get("subtype") != subtype:
return False
return True
elif isinstance(schema, list):
return any(schema_supports(s, type=type, subtype=subtype) for s in schema)
else:
raise ValueError(schema)
12 changes: 6 additions & 6 deletions openeo/rest/datacube.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
import shapely.geometry.base
from shapely.geometry import MultiPolygon, Polygon, mapping

from openeo.api.process import Parameter
from openeo.api.process import Parameter, schema_supports
from openeo.dates import get_temporal_extent
from openeo.internal.documentation import openeo_process
from openeo.internal.graph_building import PGNode, ReduceNode, _FromNodeMixin
Expand Down Expand Up @@ -182,10 +182,10 @@ def load_collection(
temporal_extent = cls._get_temporal_extent(extent=temporal_extent)

if isinstance(spatial_extent, Parameter):
if spatial_extent.schema.get("type") != "object":
if not schema_supports(spatial_extent.schema, type="object"):
warnings.warn(
"Unexpected parameterized `spatial_extent` in `load_collection`:"
f" expected schema with type 'object' but got {spatial_extent.schema!r}."
f" expected schema compatible with type 'object' but got {spatial_extent.schema!r}."
)
arguments = {
'id': collection_id,
Expand Down Expand Up @@ -481,7 +481,7 @@ def filter_bbox(
crs: Optional[Union[int, str]] = None,
base: Optional[float] = None,
height: Optional[float] = None,
bbox: Optional[Sequence[float]] = None,
bbox: Union[Sequence[float], Parameter, None] = None,
) -> DataCube:
"""
Limits the data cube to the specified bounding box.
Expand Down Expand Up @@ -555,10 +555,10 @@ def filter_bbox(
raise ValueError(args)

if isinstance(bbox, Parameter):
if bbox.schema.get("type") != "object":
if not schema_supports(bbox.schema, type="object"):
warnings.warn(
"Unexpected parameterized `extent` in `filter_bbox`:"
f" expected schema with type 'object' but got {bbox.schema!r}."
f" expected schema compatible with type 'object' but got {bbox.schema!r}."
)
extent = bbox
else:
Expand Down
34 changes: 33 additions & 1 deletion tests/api/test_process.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import pytest

from openeo.api.process import Parameter
from openeo.api.process import Parameter, schema_supports


def test_parameter_defaults():
Expand Down Expand Up @@ -261,3 +261,35 @@ def test_parameter_spatial_extent():
},
],
}


def test_schema_supports_type_basic():
schema = {"type": "string"}
assert schema_supports(schema, type="string") is True
assert schema_supports(schema, type="number") is False


def test_schema_supports_type_list():
schema = {"type": ["string", "number"]}
assert schema_supports(schema, type="string") is True
assert schema_supports(schema, type="number") is True
assert schema_supports(schema, type="object") is False


def test_schema_supports_subtype():
schema = {"type": "object", "subtype": "datacube"}
assert schema_supports(schema, type="object") is True
assert schema_supports(schema, type="object", subtype="datacube") is True
assert schema_supports(schema, type="object", subtype="geojson") is False


def test_schema_supports_list():
schema = [
{"type": "string"},
{"type": "object", "subtype": "datacube"},
]
assert schema_supports(schema, type="string") is True
assert schema_supports(schema, type="number") is False
assert schema_supports(schema, type="object") is True
assert schema_supports(schema, type="object", subtype="datacube") is True
assert schema_supports(schema, type="object", subtype="geojson") is False
52 changes: 48 additions & 4 deletions tests/rest/datacube/test_datacube100.py
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,15 @@ def test_filter_bbox_kwargs(con100: Connection, kwargs, expected):
assert node["arguments"]["extent"] == expected


def test_filter_bbox_parameter(con100: Connection):
@pytest.mark.parametrize(
"bbox_param",
[
Parameter(name="my_bbox", schema={"type": "object"}),
Parameter.spatial_extent(name="my_bbox"),
Parameter.bounding_box(name="my_bbox"),
],
)
def test_filter_bbox_parameter(con100: Connection, bbox_param):
expected = {
"process_id": "filter_bbox",
"arguments": {
Expand All @@ -252,7 +260,6 @@ def test_filter_bbox_parameter(con100: Connection):
},
"result": True,
}
bbox_param = Parameter(name="my_bbox", schema={"type": "object"})

cube = con100.load_collection("S2").filter_bbox(bbox_param)
assert _get_leaf_node(cube) == expected
Expand All @@ -274,14 +281,14 @@ def test_filter_bbox_parameter_invalid_schema(con100: Connection):

with pytest.warns(
UserWarning,
match="Unexpected parameterized `extent` in `filter_bbox`: expected schema with type 'object' but got {'type': 'string'}.",
match="Unexpected parameterized `extent` in `filter_bbox`: expected schema compatible 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'}.",
match="Unexpected parameterized `extent` in `filter_bbox`: expected schema compatible with type 'object' but got {'type': 'string'}.",
):
cube = con100.load_collection("S2").filter_bbox(bbox=bbox_param)
assert _get_leaf_node(cube) == expected
Expand Down Expand Up @@ -2301,6 +2308,43 @@ def test_load_collection_parameterized_bands(con100):
}


@pytest.mark.parametrize(
["spatial_extent", "temporal_extent", "spatial_name", "temporal_name"],
[
(
Parameter.spatial_extent(),
Parameter.temporal_interval(),
"spatial_extent",
"temporal_extent",
),
(
Parameter.bounding_box(name="spatial_extent"),
Parameter.temporal_interval(),
"spatial_extent",
"temporal_extent",
),
(
Parameter(name="my_bbox", schema={"type": "object"}),
Parameter(name="dates", schema={"type": "array"}),
"my_bbox",
"dates",
),
],
)
def test_load_collection_parameterized_extents(con100, spatial_extent, temporal_extent, spatial_name, temporal_name):
cube = con100.load_collection("S2", spatial_extent=spatial_extent, temporal_extent=temporal_extent)
assert get_download_graph(cube, drop_save_result=True) == {
"loadcollection1": {
"arguments": {
"id": "S2",
"spatial_extent": {"from_parameter": spatial_name},
"temporal_extent": {"from_parameter": temporal_name},
},
"process_id": "load_collection",
},
}


def test_apply_dimension_temporal_cumsum_with_target(con100, test_data):
cumsum = con100.load_collection("S2").apply_dimension('cumsum', dimension="t", target_dimension="MyNewTime")
actual_graph = cumsum.flat_graph()
Expand Down
56 changes: 44 additions & 12 deletions tests/rest/test_udp.py
Original file line number Diff line number Diff line change
Expand Up @@ -279,12 +279,28 @@ def test_delete(con100, requests_mock):
assert adapter.called


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")
@pytest.mark.parametrize(
["layer", "dates", "bbox"],
[
(
Parameter.string("layer", description="Collection Id"),
Parameter.string("dates", description="Temporal extent"),
Parameter("bbox", schema="object", description="bbox"),
),
(
Parameter.string("layer", description="Collection Id"),
Parameter.temporal_interval(name="dates"),
Parameter.spatial_extent(name="bbox"),
),
(
Parameter.string("layer", description="Collection Id"),
Parameter.temporal_interval(name="dates"),
Parameter.bounding_box(name="bbox"),
),
],
)
def test_build_parameterized_cube_filters(con100, layer, dates, bbox, recwarn):
cube = con100.load_collection(layer).filter_temporal(dates).filter_bbox(bbox)

assert cube.flat_graph() == {
"loadcollection1": {
"process_id": "load_collection",
Expand Down Expand Up @@ -353,12 +369,28 @@ def test_build_parameterized_cube_start_date(con100, recwarn):
assert recwarn.list == []


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")
@pytest.mark.parametrize(
["layer", "dates", "bbox"],
[
(
Parameter.string("layer", description="Collection Id"),
Parameter.string("dates", description="Temporal extent"),
Parameter("bbox", schema="object", description="bbox"),
),
(
Parameter.string("layer", description="Collection Id"),
Parameter.temporal_interval(name="dates"),
Parameter.spatial_extent(name="bbox"),
),
(
Parameter.string("layer", description="Collection Id"),
Parameter.temporal_interval(name="dates"),
Parameter.bounding_box(name="bbox"),
),
],
)
def test_build_parameterized_cube_load_collection(con100, recwarn, layer, dates, bbox):
cube = con100.load_collection(layer, spatial_extent=bbox, temporal_extent=dates)

assert cube.flat_graph() == {
"loadcollection1": {
"process_id": "load_collection",
Expand All @@ -379,7 +411,7 @@ def test_build_parameterized_cube_load_collection_invalid_bbox_schema(con100):
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'}.",
match="Unexpected parameterized `spatial_extent` in `load_collection`: expected schema compatible with type 'object' but got {'type': 'string'}.",
):
cube = con100.load_collection(layer, spatial_extent=bbox, temporal_extent=dates)

Expand All @@ -401,7 +433,7 @@ def test_build_parameterized_cube_filter_bbox_invalid_schema(con100):
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'}.",
match="Unexpected parameterized `extent` in `filter_bbox`: expected schema compatible with type 'object' but got {'type': 'string'}.",
):
cube = con100.load_collection(layer).filter_bbox(bbox)

Expand Down

0 comments on commit 179a852

Please sign in to comment.