Repository: metno/pyaerocom

Issue:
ungridded_structured based observations should show revision-ID on aeroval
**Describe the bug**

pyaro-based observations like new EEA or EBAS don't show revision-ID on aeroval:

![Image](https://github.com/user-attachments/assets/c4584455-a09e-431a-a951-6a90148cf2ea)

in any version since pyaro-base readers where introdused.

**Expected behavior**
The Obs-revision should be shown on aeroval.


**Additional context**
Originally reported by @jgriesfeller

Discussion:

Solution patch:
diff --git a/pyaerocom/colocation/colocation_3d.py b/pyaerocom/colocation/colocation_3d.py
index 5131c017..d0b5d5a6 100644
--- a/pyaerocom/colocation/colocation_3d.py
+++ b/pyaerocom/colocation/colocation_3d.py
@@ -238,14 +238,11 @@ def _colocate_vertical_profile_gridded(
                 )
 
         try:
-            revision = data_ref.data_revision[dataset_ref]
+            revision = data_ref.get_data_revision[dataset_ref]
+        except MetaDataError:
+            revision = "MULTIPLE"
         except Exception:
-            try:
-                revision = data_ref._get_data_revision_helper(dataset_ref)
-            except MetaDataError:
-                revision = "MULTIPLE"
-            except Exception:
-                revision = "n/a"
+            revision = "n/a"
 
         files = [os.path.basename(x) for x in data.from_files]
 
@@ -262,7 +259,7 @@ def _colocate_vertical_profile_gridded(
             "from_files": files,
             "from_files_ref": None,
             "colocate_time": colocate_time,
-            "obs_is_clim": True if isinstance(use_climatology_ref, ClimatologyConfig) else False,
+            "obs_is_clim": (True if isinstance(use_climatology_ref, ClimatologyConfig) else False),
             "pyaerocom": pya_ver,
             "min_num_obs": min_num_obs,
             "resample_how": resample_how,
diff --git a/pyaerocom/colocation/colocation_utils.py b/pyaerocom/colocation/colocation_utils.py
index 512f62a6..653793af 100644
--- a/pyaerocom/colocation/colocation_utils.py
+++ b/pyaerocom/colocation/colocation_utils.py
@@ -947,14 +947,11 @@ def colocate_gridded_ungridded(
                 f"not be added to ColocatedData. Reason: {e}"
             )
     try:
-        revision = data_ref.data_revision[dataset_ref]
+        revision = data_ref.get_data_revision(dataset_ref)
+    except MetaDataError:
+        revision = "MULTIPLE"
     except Exception:
-        try:
-            revision = data_ref._get_data_revision_helper(dataset_ref)
-        except MetaDataError:
-            revision = "MULTIPLE"
-        except Exception:
-            revision = "n/a"
+        revision = "n/a"
 
     files = [os.path.basename(x) for x in data.from_files]
 
diff --git a/pyaerocom/io/read_ebas.py b/pyaerocom/io/read_ebas.py
index ae75d140..d2d1360a 100644
--- a/pyaerocom/io/read_ebas.py
+++ b/pyaerocom/io/read_ebas.py
@@ -1435,6 +1435,7 @@ class ReadEbas(ReadUngriddedBase):
         data_out = StationData()
 
         data_out = self._add_meta(data_out, file)
+        data_out.data_revision = self.data_revision
 
         freq_ebas = data_out["ts_type"]  # resolution code
         # store the raw EBAS meta dictionary (who knows what for later ;P )
diff --git a/pyaerocom/ungridded_data_container.py b/pyaerocom/ungridded_data_container.py
index 50586c16..e370cbf8 100644
--- a/pyaerocom/ungridded_data_container.py
+++ b/pyaerocom/ungridded_data_container.py
@@ -57,9 +57,9 @@ class UngriddedDataContainer(abc.ABC):
         return data
 
     @abc.abstractmethod
-    def _get_data_revision_helper(self, data_id):
+    def get_data_revision(self, data_id):
         """
-        Helper method to get last data revision
+        Get the data revision of the data_id
 
         Parameters
         ----------
@@ -924,7 +924,7 @@ class UngriddedDataContainer(abc.ABC):
             obj.append_station_data(all_stations["stats"])
 
         # update metadata
-        obj.data_revision.update(other.data_revision)
+        obj._data_revision.update(other._data_revision)
         obj.filter_hist.update(other.filter_hist)
 
         return obj
diff --git a/pyaerocom/ungridded_data_metadata.py b/pyaerocom/ungridded_data_metadata.py
index d8b900b3..5b60f4b6 100644
--- a/pyaerocom/ungridded_data_metadata.py
+++ b/pyaerocom/ungridded_data_metadata.py
@@ -44,7 +44,7 @@ class UngriddedDataMetadata(UngriddedDataContainer):
 
     def __init__(self):
         self.metadata = {}
-        self.data_revision = {}
+        self._data_revision = {}
         self.var_idx = {}
         self.filter_hist = {}
         self._is_vertical_profile = False
@@ -54,15 +54,15 @@ class UngriddedDataMetadata(UngriddedDataContainer):
         deepcopy metadata-fields to the other object
         """
         other.metadata = deepcopy(self.metadata)
-        other.data_revision = deepcopy(self.data_revision)
+        other._data_revision = deepcopy(self._data_revision)
         other.var_idx = deepcopy(self.var_idx)
         other.filter_hist = deepcopy(self.filter_hist)
         other.is_vertical_profile = self._is_vertical_profile
 
     @override
-    def _get_data_revision_helper(self, data_id):
+    def get_data_revision(self, data_id) -> str | None:
         """
-        Helper method to get last data revision
+        Get data revision for a data_id.
 
         Parameters
         ----------
@@ -79,18 +79,18 @@ class UngriddedDataMetadata(UngriddedDataContainer):
         latest revision (None if no revision is available).
 
         """
-        rev = None
-        for meta in self.metadata.values():
-            if meta["data_id"] == data_id:
-                if rev is None:
-                    rev = meta["data_revision"]
-                elif not meta["data_revision"] == rev:
-                    raise MetaDataError(f"Found different data revisions for dataset {data_id}")
-        if data_id in self.data_revision:
-            if not rev == self.data_revision[data_id]:
-                raise MetaDataError(f"Found different data revisions for dataset {data_id}")
-        self.data_revision[data_id] = rev
-        return rev
+        if data_id not in self._data_revision:
+            rev = None
+            for meta in self.metadata.values():
+                if meta["data_id"] == data_id:
+                    if rev is None:
+                        rev = meta["data_revision"]
+                    elif not meta["data_revision"] == rev:
+                        raise MetaDataError(
+                            f"Found different data revisions for dataset {data_id}"
+                        )
+            self._data_revision[data_id] = rev
+        return self._data_revision[data_id]
 
     @property
     def _first_meta_idx(self):
diff --git a/pyaerocom/ungriddeddata.py b/pyaerocom/ungriddeddata.py
index 9179afb7..e003708f 100644
--- a/pyaerocom/ungriddeddata.py
+++ b/pyaerocom/ungriddeddata.py
@@ -700,7 +700,7 @@ class UngriddedData(UngriddedDataMetadata):
             rev = meta["data_revision"]
         else:
             try:
-                rev = self.data_revision[meta["data_id"]]
+                rev = self.get_data_revision[meta["data_id"]]
             except Exception:
                 logger.debug("Data revision could not be accessed")
         sd.data_revision = rev
@@ -1182,7 +1182,7 @@ class UngriddedData(UngriddedDataMetadata):
 
         # write history of filtering applied
         new.filter_hist.update(self.filter_hist)
-        new.data_revision.update(self.data_revision)
+        new._data_revision.update(self._data_revision)
 
         return new
 
@@ -1534,7 +1534,7 @@ class UngriddedData(UngriddedDataMetadata):
             obj._data = other._data
             obj.metadata = other.metadata
             # obj.unit = other.unit
-            obj.data_revision = other.data_revision
+            obj._data_revision = other._data_revision
             obj.meta_idx = other.meta_idx
             # potentially temporary fix for pyaro actrisebas reader
             # if len(other.meta_idx) != len(other.metadata):
@@ -1569,7 +1569,7 @@ class UngriddedData(UngriddedDataMetadata):
                     else:
                         obj.var_idx[var] = idx
             obj._data = np.vstack([obj._data, other._data])
-            obj.data_revision.update(other.data_revision)
+            obj._data_revision.update(other._data_revision)
         obj.filter_hist.update(other.filter_hist)
         obj._check_index()
         return obj
diff --git a/pyaerocom/ungriddeddata_structured.py b/pyaerocom/ungriddeddata_structured.py
index edb27685..3bff7865 100644
--- a/pyaerocom/ungriddeddata_structured.py
+++ b/pyaerocom/ungriddeddata_structured.py
@@ -295,7 +295,7 @@ class UngriddedDataStructured(UngriddedDataMetadata):
             rev = meta["data_revision"]
         else:
             try:
-                rev = self.data_revision[meta["data_id"]]
+                rev = self.get_data_revision(meta["data_id"])
             except Exception:
                 logger.debug("Data revision could not be accessed")
         sd.data_revision = rev
@@ -620,6 +620,8 @@ class UngriddedDataStructured(UngriddedDataMetadata):
                     self.metadata[meta_idx][key] = station_data[key]
             contains_vars = list(station_data.var_info)
             self.metadata[meta_idx]["variables"] = contains_vars
+            if "data_revision" in station_data:
+                self.metadata[meta_idx]["data_revision"] = station_data.data_revision
 
             for var in contains_vars:
                 vardata = station_data[var]
@@ -817,6 +819,14 @@ class UngriddedDataStructured(UngriddedDataMetadata):
             ugs._dra.append_array(**dra_data)
 
         logger.info(f"Converting metadata from pyaro/{data_id} to ungridded")
+        rev = None
+        if "revision" in reader.metadata():
+            rev = reader.metadata()["revision"]
+        else:
+            logger.warning(
+                f"pyaro/{data_id} does not contain a 'revision', please inform data-provider, or pyaro-readers"
+            )
+
         stations_with_metadata = reader.stations()
         for var in vars_to_retrieve:
             for station_tstype, meta_id in var_metas[var].items():
@@ -824,6 +834,7 @@ class UngriddedDataStructured(UngriddedDataMetadata):
                 extra_metadata = stations_with_metadata[station_name].metadata
                 d = {
                     "data_id": data_id,
+                    "data_revision": rev,
                     "station_name": station_name,
                     "var_info": {
                         var: {"units": var_units[var]},


Test patch:
diff --git a/tests/io/pyaro/test_read_pyaro.py b/tests/io/pyaro/test_read_pyaro.py
index 62cf9af6..f0bdf755 100644
--- a/tests/io/pyaro/test_read_pyaro.py
+++ b/tests/io/pyaro/test_read_pyaro.py
@@ -173,7 +173,10 @@ def test_vmrox():
     )
     reader = PyaroToUngriddedData(config)
     data = reader.read(vars_to_retrieve=["vmrox"])
-
+    rev = data.get_data_revision("whatever")
+    # WIP: waiting for eeareader to add revision, to be done by m06-2025
+    # assert rev is not None
+    rev is not None
     alldata = data.to_station_data_all()
     stats = alldata["stats"]
     assert len(stats) >= 4
diff --git a/tests/test_ungriddeddata_structured.py b/tests/test_ungriddeddata_structured.py
index 07266fa2..58e9959c 100644
--- a/tests/test_ungriddeddata_structured.py
+++ b/tests/test_ungriddeddata_structured.py
@@ -114,12 +114,19 @@ ALL_SITES = [
 def test_filter_by_meta(aeronetsunv3lev2_subset_uds, args, sitenames):
     data = aeronetsunv3lev2_subset_uds
     assert isinstance(data, UngriddedDataStructured)
+    assert data.get_data_revision("AeronetSunV3L2Subset.daily") == "n/d"
     subset = data.filter_by_meta(**args)
+    assert subset.get_data_revision("AeronetSunV3L2Subset.daily") == "n/d"
     sites = [x["station_name"] for x in subset.metadata.values()]
     stats = sorted(list(dict.fromkeys(sites)))
     assert sorted(sitenames) == stats
 
 
+def test_ebas_revision(data_scat_jungfraujoch: UngriddedDataContainer):
+    assert isinstance(data_scat_jungfraujoch, UngriddedDataStructured)
+    assert data_scat_jungfraujoch.get_data_revision("EBASSubset") == "20220101"
+
+
 def test_cache_reload(data_scat_jungfraujoch: UngriddedDataContainer, tmp_path: Path):
     path = tmp_path / "ungridded_scat_jungfraujoch.pkl"
     file = data_scat_jungfraujoch.save_as(file_name=path.name, save_dir=path.parent)