Merge branch 'issue949-cube-metadata2'

Author: soxofaan

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
 

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
         )

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=[]),

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

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")

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:

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"]