Respect STAC cube:dimensions as source of truth in metadata_from_stac + align tests with dimension policy by suriyahgit · Pull Request #867 · Open-EO/openeo-python-client

suriyahgit · 2026-03-03T10:56:36Z

Problem

metadata_from_stac() previously injected openEO default dimensions (x, y, t, bands) even when a STAC object already defined dimensions via cube:dimensions.

This led to:

dimension names being overridden or altered,
inconsistencies between STAC metadata and openEO metadata,
tests relying on implicit default behavior instead of STAC semantics.

Fix

Introduce a clear dimension resolution policy:

If cube:dimensions is present
→ treat it as the source of truth and preserve dimension names and order.
If cube:dimensions is absent
→ fall back to openEO defaults (x, y, optional t).

The metadata parsing logic was refactored accordingly and aligned with PySTAC datacube capabilities.

Testing

Updated existing tests to no longer assume an always-present band dimension.
Added checks verifying:
- fallback behavior when cube:dimensions is missing,
- preservation of dimension names when cube:dimensions exists,
- correct temporal dimension handling across STAC object types.
Tests now explicitly enforce the new dimension policy to prevent regressions.

This change makes metadata parsing more STAC-compliant while keeping backward-compatible fallback behavior.

Refer:
https://github.com/destine-datalake-cube/dedl-openeo-coordination/issues/3
Also solves: #743

…legacy unit tests fail

soxofaan

just could give this a quick scan to make some notes

…se-correction for Dict to support python3.8 Co-authored-by: Stefaan Lippens <soxofaan@users.noreply.github.com>

suriyahgit · 2026-03-10T13:31:30Z

ready for review @soxofaan

clausmichele · 2026-04-08T15:05:47Z

Hi @soxofaan , please let us know if there is something we can do to move on with this PR!

jdries · 2026-04-08T15:44:09Z

+
+# Dimension name resolution policy (STAC cube:dimensions vs openEO defaults)
+@pytest.mark.parametrize(
+    ["stac_dict", "expected_dims"],


can a STAC 1.1 style test case be added which uses the new 'bands' common metadata and no datacube extension?
Collection metadata example:
https://github.com/radiantearth/stac-spec/blob/master/examples/collection-only/collection.json#L152
Bands spec:
https://github.com/radiantearth/stac-spec/blob/master/commons/common-metadata.md#band-object

Added a STAC 1.1 bands common metadata test without the datacube extension in test_metadata_from_stac_stac_1_1_common_bands_without_datacube_extension. The test verifies that metadata_from_stac() still builds the fallback openEO dimensions and extracts the common bands metadata correctly.

jdries

FYI, Stefaan is on holiday
Added one comment myself.
Please also add a changelog entry.
Otherwise I'm inclined to say it looks good, assuming this change mostly preserves existing behaviour. (Biggest worry are collections with faulty datacube extension metadata that would suddenly have a change in behavior.)

soxofaan

some more notes

soxofaan · 2026-04-21T12:05:37Z

+            TemporalDimension(name="t", extent=self.infer_temporal_extent(stac_object)),
+        ]
+        if isinstance(stac_object, (pystac.Collection, pystac.Item)):
+            dimensions.append(BandDimension(name="bands", bands=list(bands)))


why conditionally add the band dimension here?

Removed this conditional injection. When cube:dimensions is present, the declared dimensions are now treated as the source of truth and no fallback bands dimension is added.

soxofaan · 2026-04-21T12:09:32Z

+        cube_dimensions = self.cube_dimensions_dict(stac_object)
+        return isinstance(cube_dimensions, dict) and len(cube_dimensions) > 0
+
+    def cube_dimensions_dict(self, stac_object: pystac.STACObject) -> Dict[str, dict]:


I don't think this has to be in the public interface

Agreed. I made these parser helpers private: _has_cube_dimensions(), _cube_dimensions_dict(), and _parse_declared_dimensions().

soxofaan · 2026-04-21T12:14:38Z

+            return [Rfc3339(propagate_none=True).normalize(d) for d in interval]
+
+        if isinstance(stac_object, pystac.Item):
+            props = getattr(stac_object, "properties", {}) or {}


just wondering: why using getattr here? Isn't the presence of properties guaranteed?

Changed this to use stac_obj.properties directly for items.

soxofaan · 2026-04-21T12:17:01Z

+                norm = Rfc3339(propagate_none=True).normalize(dt_)
+                return [norm, norm]
+            start = props.get("start_datetime")
+            end = props.get("end_datetime")


isn't the "datetime" field a required field, so is this code even reachable?
Or to put differently: shouldn't "start_datetime"/"end_datetime" get higher precedence over "datetime"?

updated the item temporal fallback logic so start_datetime / end_datetime take precedence over datetime and added test coverage for that precedence.

soxofaan · 2026-04-21T12:20:22Z

+        dimensions = self.dimensions_from_stac_object(stac_object=stac_object, bands=bands)
+        return CubeMetadata(dimensions=dimensions)
+
+    def dimensions_from_stac_object(self, stac_object: pystac.STACObject, bands: _BandList) -> List[Dimension]:


Why having bands as argument here? Shouldn't it be the responsibility of this method to detect the bands? Note that this is a public method, so it's weird to have an argument for something you actually want to extract

Agreed. dimensions_from_stac_object() now detects bands internally through bands_from_stac_object() instead of requiring a bands argument. I didn't notice that this bands_from stac_object() exists earlier, my bad.

soxofaan · 2026-04-21T12:22:10Z

+        dimensions: List[Dimension] = [
+            SpatialDimension(name="x", extent=[None, None]),
+            SpatialDimension(name="y", extent=[None, None]),
+            TemporalDimension(name="t", extent=self.infer_temporal_extent(stac_object)),


reuse get_temporal_dimension here instead ?

removed the usage of this temporal extent helper, and reusing get_temporal_dimension here instead.

soxofaan · 2026-04-21T12:23:36Z

+
+        return ext or [None, None]
+
+    def parse_declared_dimensions(self, stac_object: pystac.STACObject, bands: _BandList) -> List[Dimension]:


as noted above, it looks weird to have this required bands argument here without explanation

Removed the required bands argument from the public-facing parser method. dimensions_from_stac_object() now detects bands internally through bands_from_stac_object(), no longer needed to pass extracted bands into the method. This behaviour is consistent across PR now!

clausmichele · 2026-07-15T09:11:05Z

@soxofaan @jdries this PR is ready to review when you can! This issue reminded me about this pending PR: https://forum.dataspace.copernicus.eu/t/load-stac-time-dimension-hidden/5312/3

pjonsson · 2026-07-16T10:39:22Z

+        cube_dimensions = self._cube_dimensions_dict(stac_object)
+        return isinstance(cube_dimensions, dict) and len(cube_dimensions) > 0
+
+    def _cube_dimensions_dict(self, stac_object: pystac.STACObject) -> Dict[str, dict]:


Can this be a static method?

You should probably check if there are other methods added by this PR that could also be made static as consequence of making this method static.

pjonsson · 2026-07-16T10:44:50Z

+        PySTAC cube dimension wrapper may raise if 'extent' is missing.
+        Also, depending on serialization/version, extent might live in extra_fields.
+        """
+        try:


Can this try-block be replaced by:

ext = getattr(dim, "extent", None)

and the code will do the same thing?

pjonsson · 2026-07-16T10:46:53Z

+        """
+        try:
+            ext = dim.extent
+        except Exception:


If the getattr() I suggested in the previous comment isn't right, catching Exception here is too broad, the code should catch the exception it's expecting and let other exceptions through so the caller can deal with unexpected errors.

pjonsson · 2026-07-16T10:48:13Z

+            _PYSTAC_1_9_EXTENSION_INTERFACE
+            and getattr(stac_object, "ext", None) is not None
+            and stac_object.ext.has("cube")
+            and hasattr(stac_object.ext, "cube")


Does this line do the same as the previous line?

pjonsson · 2026-07-16T10:49:50Z

+    def _parse_cube_dimensions_from_raw_dict(self, stac_object: pystac.STACObject, bands: _BandList) -> List[Dimension]:
+        """
+        Parse dimensions from raw cube:dimensions dict.
+        Supports 'spatial', 'temporal', and ('bands' or 'spectral' as an alias).


Parenthesis error? The sentence ends with an "and".

pjonsson · 2026-07-16T10:53:20Z

+        return {}
+
+    @staticmethod
+    def _safe_extent_from_pystac_cube_dim(dim) -> list:


Can't use list in Python 3.8.

And since the old code is at least partially annotated, I think it's nice if you add type annotations for the new functions/methods in this PR.

suriyahgit and others added 5 commits February 24, 2026 17:27

solving the issue Open-EO#743, cube:dimension and fallback

ccd7f23

temporal policy tests modified along with cube:dimension

b49026b

lint run successfull

b9abf3b

rolling back changes to the band dimension when left empty - causing …

d83208c

…legacy unit tests fail

Merge branch 'Open-EO:master' into dim_policy

f77982c

clausmichele mentioned this pull request Mar 6, 2026

Improve bands handling in load_stac Eurac-Research-Institute-for-EO/openeo-processes-dask#4

Closed

Yuvraj198920 mentioned this pull request Mar 6, 2026

Refactor load_collection to use load_stac from openeo-processes-dask Eurac-Research-Institute-for-EO/openeo-argoworkflows#22

Open

soxofaan reviewed Mar 9, 2026

View reviewed changes

suriyahgit and others added 2 commits March 10, 2026 10:10

Apply suggestions from code review - Assertion message removal and ca…

ad706de

…se-correction for Dict to support python3.8 Co-authored-by: Stefaan Lippens <soxofaan@users.noreply.github.com>

solving reviews flagged by soxofaan in the PR Open-EO#867

833d1e7

jdries reviewed Apr 8, 2026

View reviewed changes

jdries requested changes Apr 8, 2026

View reviewed changes

soxofaan reviewed Apr 21, 2026

View reviewed changes

suriyahgit and others added 3 commits April 21, 2026 16:56

Merge branch 'Open-EO:master' into dim_policy

64a1423

Merge branch 'Open-EO:master' into dim_policy

2dd590d

Address STAC dimension policy review feedback

954fe02

suriyahgit requested review from jdries and soxofaan May 15, 2026 10:51

Merge branch 'master' into dim_policy

6206bff

pjonsson reviewed Jul 16, 2026

View reviewed changes


		return ext or [None, None]

		def parse_declared_dimensions(self, stac_object: pystac.STACObject, bands: _BandList) -> List[Dimension]:

Uh oh!

Conversation

suriyahgit commented Mar 3, 2026

Problem

Fix

Testing

Uh oh!

soxofaan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

suriyahgit commented Mar 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

clausmichele commented Apr 8, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jdries left a comment

Choose a reason for hiding this comment

Uh oh!

soxofaan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

clausmichele commented Jul 15, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

suriyahgit commented Mar 10, 2026 •

edited

Loading