From 9d7171181fcdad9c68dd92c5b3fd4c2ddc3fdcab Mon Sep 17 00:00:00 2001 From: Stefaan Lippens Date: Tue, 6 Oct 2026 18:53:20 +0200 Subject: [PATCH] Issue #949 promote CubeMetadata usage eliminate improper usage of CollectionMetadata --- CHANGELOG.md | 3 +++ .../spectral_indices/spectral_indices.py | 21 ++++++++++++++----- openeo/local/connection.py | 9 ++------ openeo/metadata/__init__.py | 13 +++++++++++- openeo/rest/datacube.py | 18 ++++++++++------ openeo/rest/vectorcube.py | 11 +++++----- openeo/udf/udf_signatures.py | 8 +++---- 7 files changed, 54 insertions(+), 29 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 563622664..47c6ed488 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - Ranged downloads (e.g. of batch job results) no longer implement their own retry loop; transient failures are retried by the standard urllib3 retry configuration of the connection's session ([#934](https://github.com/Open-EO/openeo-python-client/issues/934)) +- Replaced improper `CollectionMetadata` usage with `CubeMetadata`. `CollectionMetadata` describes the metadata of a whole openEO/STAC collection, while `CubeMetadata` describes the specific dimension metadata of a concrete cube being processed. (Related to [464](https://github.com/Open-EO/openeo-python-client/issues/464), [#949](https://github.com/Open-EO/openeo-python-client/issues/949), [#827](https://github.com/Open-EO/openeo-python-client/issues/827).) + - The `metadata` attribute of `DataCube`/`VectorCube` is now a `CubeMetadata` object instead of misleading `CollectionMetadata`. + - The `apply_metadata()` UDF signature changed to use `CubeMetadata` as input and output type annotation. ### Removed diff --git a/openeo/extra/spectral_indices/spectral_indices.py b/openeo/extra/spectral_indices/spectral_indices.py index 94548e562..4231b09a5 100644 --- a/openeo/extra/spectral_indices/spectral_indices.py +++ b/openeo/extra/spectral_indices/spectral_indices.py @@ -2,10 +2,9 @@ import importlib.resources import json import re -from typing import Dict, List, Optional, Set +from typing import Dict, Iterable, List, Optional, Set from openeo import BaseOpenEoException -from openeo.metadata import CollectionMetadata from openeo.processes import ProcessBuilder, array_create, array_modify from openeo.rest.datacube import DataCube @@ -211,6 +210,14 @@ def _callback( return array_create(data=index_values) +def _extract_collection_ids(cube: DataCube) -> Iterable[str]: + """Determine the collection ids used in load_collection nodes in the process graph.""" + for node in cube.result_node().walk_nodes(): + if node.process_id == "load_collection" and isinstance(node.arguments.get("id"), str): + yield node.arguments["id"] + # TODO: Possible to determing/guess collection id or name from `load_stac` url (without requesting it)? + + def compute_and_rescale_indices( datacube: DataCube, index_dict: dict, @@ -270,10 +277,14 @@ def compute_and_rescale_indices( # Automatic band mapping band_mapping = _BandMapping() if platform is None: - if isinstance(datacube.metadata, CollectionMetadata) and datacube.metadata.get("id"): - platform = band_mapping.guess_platform(name=datacube.metadata.get("id")) + collection_ids = set(_extract_collection_ids(cube=datacube)) + platform_guesses = set(band_mapping.guess_platform(name=cid) for cid in collection_ids) + if len(platform_guesses) == 1: + [platform] = platform_guesses else: - raise BandMappingException("Unable to determine satellite platform from data cube metadata") + raise BandMappingException( + f"Unable to guess satellite platform from {collection_ids=} ({platform_guesses=})" + ) band_to_var = band_mapping.actual_band_name_to_variable_map( platform=platform, band_names=datacube.metadata.band_names ) diff --git a/openeo/local/connection.py b/openeo/local/connection.py index 7de3cd452..cfe9a9345 100644 --- a/openeo/local/connection.py +++ b/openeo/local/connection.py @@ -21,6 +21,7 @@ Band, BandDimension, CollectionMetadata, + CubeMetadata, SpatialDimension, TemporalDimension, ) @@ -233,13 +234,7 @@ def load_stac( if temporal_extent is not None: arguments["temporal_extent"] = TemporalInterval.parse_obj(temporal_extent) xarray_cube = load_stac(**arguments) - attrs = xarray_cube.attrs - for at in attrs: - # allowed types: str, Number, ndarray, number, list, tuple - if not isinstance(attrs[at], (int, float, str, np.ndarray, list, tuple)): - attrs[at] = str(attrs[at]) - metadata = CollectionMetadata( - attrs, + metadata = CubeMetadata( dimensions=[ SpatialDimension(name=xarray_cube.openeo.x_dim, extent=[]), SpatialDimension(name=xarray_cube.openeo.y_dim, extent=[]), diff --git a/openeo/metadata/__init__.py b/openeo/metadata/__init__.py index e730040b8..b203bed06 100644 --- a/openeo/metadata/__init__.py +++ b/openeo/metadata/__init__.py @@ -544,9 +544,15 @@ class CollectionMetadata(CubeMetadata): Metadata is expected to follow format defined by https://openeo.org/documentation/1.0/developers/api/reference.html#operation/describe-collection (with partial support for older versions) - """ + # TODO #949 Fully decouple CollectionMetadata from CubeMetadata. + # In the beginning, there was only CollectionMetadata, and later CubeMetadata was inserted as quickfix. + # Superficially there are similarities, but this coupled design is becoming counterproductive. + # CollectionMetadata generally describes an external, immutable (STAC) resource, + # while CubeMetadata is for keeping track of important dimension/metadata aspects during cube manipulations. + # Problem: openeo-geopyspark-driver still heavily depends on CollectionMetadata acting as cube metadata. + def __init__(self, metadata: dict, dimensions: List[Dimension] = None, _federation: Optional[dict] = None): self._orig_metadata = metadata if dimensions is None: @@ -572,6 +578,7 @@ def _parse_dimensions(cls, spec: dict, complain: Callable[[str], None] = _log.wa :return list: list of `Dimension` objects """ + # TODO: can this whole parse logic be replaced with _StacMetadataParser/metadata_from_stac logic? # Dimension info is in `cube:dimensions` (or 0.4-style `properties/cube:dimensions`) cube_dimensions = ( @@ -654,6 +661,10 @@ def _clone_and_update( This overrides the method in `CubeMetadata` to keep the original metadata. """ + # TODO #949 there are multiple (indications of) design errors here: + # - the signature is not compatible with base interface + # - at client side, collection metadata is an external resource and immutable client side + # supporting a transform to a new "CollectionMetadata" object makes little sense cls = type(self) if metadata is None: metadata = self._orig_metadata diff --git a/openeo/rest/datacube.py b/openeo/rest/datacube.py index 0e31ccd0f..7dbee3746 100644 --- a/openeo/rest/datacube.py +++ b/openeo/rest/datacube.py @@ -48,7 +48,6 @@ from openeo.internal.warnings import UserDeprecationWarning, deprecated, legacy_alias from openeo.metadata import ( Band, - CollectionMetadata, CubeMetadata, metadata_from_stac, ) @@ -106,10 +105,14 @@ class DataCube(_ProcessGraphAbstraction): _DEFAULT_RASTER_FORMAT = "GTiff" def __init__( - self, graph: PGNode, connection: Optional[Connection] = None, metadata: Optional[CollectionMetadata] = None + self, + graph: PGNode, + *, + connection: Optional[Connection] = None, + metadata: Optional[CubeMetadata] = None, ): super().__init__(pgnode=graph, connection=connection) - self.metadata: Optional[CollectionMetadata] = metadata + self.metadata: Optional[CubeMetadata] = metadata def process( self, @@ -137,7 +140,7 @@ def process( graph_add_node = legacy_alias(process, "graph_add_node", since="0.1.1") - def process_with_node(self, pg: PGNode, metadata: Optional[CollectionMetadata] = None) -> DataCube: + def process_with_node(self, pg: PGNode, metadata: Optional[CubeMetadata] = None) -> DataCube: """ Generic helper to create a new DataCube by applying a process (given as process graph node) @@ -234,8 +237,11 @@ def load_collection( } if isinstance(collection_id, Parameter): fetch_metadata = False - metadata: Optional[CollectionMetadata] = ( - connection.collection_metadata(collection_id) if connection and fetch_metadata else None + metadata: Optional[CubeMetadata] = ( + # TODO: eliminate `.collection_metadata()._dimensions` hack and directly parse metadata like load_stac? + CubeMetadata(dimensions=connection.collection_metadata(collection_id)._dimensions) + if (connection and fetch_metadata) + else None ) if bands is not None: bands = cls._get_bands(bands, process_id="load_collection") diff --git a/openeo/rest/vectorcube.py b/openeo/rest/vectorcube.py index 921cae94e..1a22c4980 100644 --- a/openeo/rest/vectorcube.py +++ b/openeo/rest/vectorcube.py @@ -13,7 +13,7 @@ from openeo.internal.documentation import openeo_process from openeo.internal.graph_building import PGNode from openeo.internal.warnings import legacy_alias -from openeo.metadata import CollectionMetadata, CubeMetadata, Dimension +from openeo.metadata import CubeMetadata, Dimension from openeo.rest import ( DEFAULT_JOB_STATUS_POLL_CONNECTION_RETRY_INTERVAL, DEFAULT_JOB_STATUS_POLL_INTERVAL_MAX, @@ -53,20 +53,19 @@ def __init__(self, graph: PGNode, connection: Union[Connection, None], metadata: self.metadata = metadata @classmethod - def _build_metadata(cls, add_properties: bool = False) -> CollectionMetadata: - """Helper to build a (minimal) `CollectionMetadata` object.""" + def _build_metadata(cls, add_properties: bool = False) -> CubeMetadata: + """Helper to build a (minimal) `CubeMetadata` object.""" # Vector cubes have at least a "geometry" dimension dimensions = [Dimension(name="geometry", type="geometry")] if add_properties: dimensions.append(Dimension(name="properties", type="other")) - # TODO #464: use a more generic metadata container than "collection" metadata - return CollectionMetadata(metadata={}, dimensions=dimensions) + return CubeMetadata(dimensions=dimensions) def process( self, process_id: str, arguments: dict = None, - metadata: Optional[CollectionMetadata] = None, + metadata: Optional[CubeMetadata] = None, namespace: Optional[str] = None, **kwargs, ) -> VectorCube: diff --git a/openeo/udf/udf_signatures.py b/openeo/udf/udf_signatures.py index dafd4d4f2..740033a25 100644 --- a/openeo/udf/udf_signatures.py +++ b/openeo/udf/udf_signatures.py @@ -10,7 +10,7 @@ from deprecated import deprecated from pandas import Series -from openeo.metadata import CollectionMetadata +from openeo.metadata import CubeMetadata from openeo.udf.udf_data import UdfData from openeo.udf.xarraydatacube import XarrayDataCube @@ -79,7 +79,7 @@ def apply_udf_data(data: UdfData): pass -def apply_metadata(metadata: CollectionMetadata, context: dict) -> CollectionMetadata: +def apply_metadata(metadata: CubeMetadata, context: dict) -> CubeMetadata: """ .. warning:: This signature is not yet fully standardized and subject to change. @@ -93,7 +93,7 @@ def apply_metadata(metadata: CollectionMetadata, context: dict) -> CollectionMet This function does not need to be provided when using the UDF in combination with processes that by design have a clear effect on cube metadata, such as :py:meth:`~openeo.rest.datacube.DataCube.reduce_dimension()` - :param metadata: the collection metadata of the input data cube + :param metadata: the metadata of the input data cube :param context: A dictionary containing user context. :return: output metadata: the expected metadata of the cube, after applying the udf @@ -103,7 +103,7 @@ def apply_metadata(metadata: CollectionMetadata, context: dict) -> CollectionMet An example for a UDF that is applied on the 'bands' dimension, and returns a new set of bands with different labels. - >>> def apply_metadata(metadata: CollectionMetadata, context: dict) -> CollectionMetadata: + >>> def apply_metadata(metadata: CubeMetadata, context: dict) -> CubeMetadata: ... return metadata.rename_labels( ... dimension="bands", ... target=["computed_band_1", "computed_band_2"]