Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ and start a new "In Progress" section above it.

- Include `job_options` as top-level properties in `GET /jobs/{job_id}` response ([#470](https://github.com/Open-EO/openeo-python-driver/issues/470))
- Bump STAC version from `0.9.0` to `1.0.0` in capabilities endpoint, collection metadata, job results and ML model metadata ([#363](https://github.com/Open-EO/openeo-python-driver/issues/363))
- `load_collection`: add check on malformed `spatial_extent`. ([#284](https://github.com/Open-EO/openeo-python-driver/issues/284))


## 0.139.0
Expand Down
42 changes: 39 additions & 3 deletions openeo_driver/processgraph/process_implementations/io.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ def _extract_temporal_extent(args: ProcessArgs, field="extent", process_id="filt


def _extract_bbox_extent(args: ProcessArgs, field="extent", process_id="filter_bbox", handle_geojson=False) -> dict:
extent = args.get_required(name=field)
extent = args.get_required(name=field, expected_type=dict)
if handle_geojson and extent.get("type") in [
"Polygon", "MultiPolygon", "GeometryCollection", "Feature", "FeatureCollection",
]:
Expand All @@ -79,8 +79,44 @@ def _extract_bbox_extent(args: ProcessArgs, field="extent", process_id="filter_b
)
d = {"west": w, "south": s, "east": e, "north": n, "crs": "EPSG:4326"}
elif all(k in extent for k in ["west", "south", "east", "north"]):
d = {k: extent[k] for k in ["west", "south", "east", "north"]}
crs = extent.get("crs") or "EPSG:4326"
d = {}
for key in ["west", "south", "east", "north"]:
value = extent[key]
if isinstance(value, bool) or not isinstance(value, (int, float)):
raise ProcessParameterInvalidException(
parameter=field,
process=process_id,
reason=f"'{key}' must be a number, but got {value!r}.",
)
d[key] = value

if d["west"] >= d["east"]:
raise ProcessParameterInvalidException(
parameter=field,
process=process_id,
reason=f"'west' must be smaller than 'east', but got west={d['west']!r} and east={d['east']!r}.",
Comment on lines +93 to +97
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is going to break anti-meridian use cases

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shall I delete the check, or do you have an example of such a use case?

)
if d["south"] >= d["north"]:
raise ProcessParameterInvalidException(
parameter=field,
process=process_id,
reason=f"'south' must be smaller than 'north', but got south={d['south']!r} and north={d['north']!r}.",
)

crs = extent.get("crs")
if crs is None and (
d["west"] < -360 or d["west"] > 360 or d["east"] < -360 or d["east"] > 360
or d["south"] < -100 or d["south"] > 100 or d["north"] < -100 or d["north"] > 100
):
raise ProcessParameterInvalidException(
parameter=field,
process=process_id,
reason=(
"coordinates are outside the valid EPSG:4326 range while no 'crs' was specified. "
"Specify the correct 'crs' explicitly."
),
)
crs = crs or "EPSG:4326"
if isinstance(crs, int):
crs = "EPSG:{crs}".format(crs=crs)
d["crs"] = crs
Expand Down
62 changes: 62 additions & 0 deletions tests/test_views_validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,3 +49,65 @@ def test_load_collection_basic(api100, backend_implementation):
res = api100.validation(pg)
errors = res.json["errors"]
assert errors == [{"code": "MissingProduct", "message": "Tile 4322 not available"}]


@pytest.mark.parametrize(
["spatial_extent", "expected_message_part"],
[
([1, 2, 3, 4], "Expected dictionary/mapping but got"),
(
{"west": [0], "south": 60.11, "east": 25.24, "north": 60.35},
"'west' must be a number, but got [0].",
),
(
{"west": 5, "south": 51.215, "east": 4, "north": 51.22},
"'west' must be smaller than 'east'",
),
(
{"west": 4, "south": 51.22, "east": 5, "north": 51.215},
"'south' must be smaller than 'north'",
),
(
{
"west": 4329317.717132108,
"east": 4330615.2810456185,
"north": 3005295.0854642093,
"south": 3003997.791438847,
},
"outside the valid EPSG:4326 range while no 'crs' was specified",
),
],
)
def test_validation_load_collection_invalid_spatial_extent(api100, spatial_extent, expected_message_part):
pg = {
"lc": {
"process_id": "load_collection",
"arguments": {"id": "S2_FOOBAR", "spatial_extent": spatial_extent},
"result": True,
}
}
res = api100.validation(pg)
errors = res.json["errors"]
assert len(errors) == 1
assert errors[0]["code"] == "ProcessParameterInvalid"
assert expected_message_part in errors[0]["message"]


@pytest.mark.parametrize(
"spatial_extent",
[
{"west": -200, "south": 51.215, "east": -190, "north": 51.22},
{"west": 4, "south": 95, "east": 5, "north": 96},
],
)
def test_validation_load_collection_spatial_extent_lenient_epsg4326_default_bounds(api100, spatial_extent):
pg = {
"lc": {
"process_id": "load_collection",
"arguments": {"id": "S2_FOOBAR", "spatial_extent": spatial_extent},
"result": True,
}
}
res = api100.validation(pg)
errors = res.json["errors"]
assert all(error["code"] != "ProcessParameterInvalid" for error in errors)