Skip to content

Only do geometries.geoms when the attribute is present. #520

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
EmileSonneveld wants to merge 2 commits into
base: master
Choose a base branch
from
+28 −1
Open

Only do geometries.geoms when the attribute is present. #520

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f0e2b32
Only do geometries.geoms when the attribute is present. https://githu…
EmileSonneveld Jun 25, 2026
3c79546
Add test_aggregate_polygon_result_vector_cube_no_ids_to_netcdf
EmileSonneveld Jun 25, 2026
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
3 changes: 2 additions & 1 deletion openeo_driver/save_result.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -446,7 +446,8 @@ def _create_point_timeseries_xarray(self, feature_ids, timestamps, lats, lons, a

def to_netcdf(self, destination: Optional[str] = None) -> str:
def features_ids_from_index(geometries):
return ["feature_%d" % i for i in range(len(geometries.geoms))]
geoms = geometries.geoms if hasattr(geometries, "geoms") else geometries
return ["feature_%d" % i for i in range(len(geoms))]

@soxofaan soxofaan Jun 25, 2026 •
edited
Loading

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.

features_ids_from_index is already a bit obscure because of lack of type annotations. Adding this if here makes it worse I think

I would fully eliminate features_ids_from_index and just do the list comprehension inline in the if-else below. Something like

        if isinstance(self._regions, GeometryCollection):
            ...
            feature_ids = ["feature_%d" % i for i in range(len(self._regions))]
        elif isinstance(self._regions, DriverVectorCube):
            ...
            feature_ids = (list(feature_ids) if feature_ids is not None
                           else ["feature_%d" % i for i in range(self._regions.geometry_count())])
        else:
            raise ValueError(self._regions)

(Note: GeometryCollection is something we should get rid of, so this would prepare more cleanly for that)


if isinstance(self._regions, GeometryCollection):
points = [r.representative_point() for r in self._regions.geoms]
Expand Down
26 changes: 26 additions & 0 deletions tests/test_save_result_netcdf.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
from pathlib import Path

import geopandas as gpd
import numpy as np
import pytest
import xarray as xr
from numpy.testing import assert_array_equal, assert_allclose
from openeo_driver.datacube import DriverVectorCube
from openeo_driver.save_result import AggregatePolygonResult, AggregatePolygonResultCSV
from shapely.geometry import GeometryCollection, Polygon

Expand Down Expand Up @@ -150,3 +152,27 @@ def test_aggregate_polygon_result_CSV(tmp_path):
timeseries_ds.red.sel( t='2017-09-05')

assert_allclose( timeseries_ds.red.sel(feature=2).sel( t='2017-09-06').data,4645.719597)


def test_aggregate_polygon_result_vector_cube_no_ids_to_netcdf(tmp_path):
"""Test for https://github.com/Open-EO/openeo-python-driver/issues/519"""
timeseries = {
"2020-06-01T00:00:00Z": [[77.2], [77.15]],
"2020-06-02T00:00:00Z": [[77.21], [77.16]],
}
# GeoDataFrame without an 'id' column so that get_ids() returns None
gdf = gpd.GeoDataFrame(
geometry=[
Polygon([(77.11, 28.56), (77.29, 28.56), (77.29, 28.69), (77.11, 28.69)]),
Polygon([(77.11, 28.56), (77.20, 28.56), (77.20, 28.62), (77.11, 28.62)]),
]
)
regions = DriverVectorCube(gdf)

result = AggregatePolygonResult(timeseries, regions=regions)
filename = result.to_netcdf(tmp_path / "timeseries_vector_cube_no_ids.nc")

timeseries_ds = xr.open_dataset(filename)
# Feature IDs should be auto-generated as "feature_0", "feature_1", ...
assert list(timeseries_ds.coords["feature_names"].data) == ["feature_0", "feature_1"]
assert_array_equal(timeseries_ds.band_0.sel(feature=0).data, [77.2, 77.21])