Drop model-native variable attributes from CMORised output

src/access_moppy/atmosphere.py

@@ -431,15 +431,19 @@ def update_attributes(self):
         if not cmor_attrs.get("positive"):
             self.ds[self.cmor_name].attrs.pop("positive", None)
 
-        # Drop model-native attributes inherited from the source variable via the
-        # rename above that have no place in CMIP6 output:
-        #  - grid_mapping: a regular lat-lon CMIP6 grid carries none, and its
-        #    container variable is not carried into the output, so the attribute
-        #    is a dangling reference that fails the CF grid-mapping check.
-        #  - um_stash_source: a UM STASH provenance code, absent from the
-        #    published reference.
+        # Drop a model-native attribute inherited from the source variable via
+        # the rename above that has no place in CMIP output: a regular lat-lon
+        # CMIP grid carries no grid_mapping, and its container variable is not
+        # carried into the output, so the attribute is a dangling reference
+        # that fails the CF grid-mapping check. It stays here rather than
+        # joining _MODEL_NATIVE_VARIABLE_ATTRIBUTES because a model on a
+        # projected grid would legitimately need it.
+        #
+        # um_stash_source used to be popped here too. It is now dropped by
+        # _drop_model_native_attributes at the end of this method, which also
+        # reaches the orog that model-level files carry as a formula_terms
+        # target — where this pop, scoped to cmor_name, never did.
         self.ds[self.cmor_name].attrs.pop("grid_mapping", None)
-        self.ds[self.cmor_name].attrs.pop("um_stash_source", None)
 
         # CMIP7 tables don't carry a per-variable "type" (unlike CMIP6), so
         # falling back to a hardcoded "double" here silently upcasts every
@@ -611,3 +615,8 @@ def update_attributes(self):
         # CF-1.11 units_metadata for the temperature and time units, last so it
         # sees the final variable units and the normalized calendar.
         self._apply_units_metadata()
+
+        # Last of all: strip the ACCESS-native attributes the raw files carry.
+        # After _check_calendar, which reads and rewrites calendar_type, and
+        # after every step above that sets attributes of its own.
+        self._drop_model_native_attributes()

src/access_moppy/base.py

@@ -1692,6 +1692,68 @@ def _drop_stale_range_attributes(self, cmor_attrs: Dict[str, Any]):
             if cmor_attrs.get(attr) in (None, ""):
                 attrs.pop(attr, None)
 
+    #: Attributes the raw model output carries that describe how ACCESS wrote a
+    #: field, not what the field is. Unlike the CMOR table directives filtered
+    #: by :data:`_CMOR_VARIABLE_ATTRIBUTES`, these are already on the variable
+    #: by the time the vocabulary is applied — they arrive with the source file
+    #: — so an allowlist on the table entry cannot reach them and they have to
+    #: be named for removal. The set is empirical: it is every non-CF attribute
+    #: found on a variable across the 30,373 CMORised ACCESS-ESM1-6 files under
+    #: /scratch/p73/ESM1p6_CMORised, which covers all three components.
+    #:
+    #: * ``time_avg_info``, ``cartesian_axis``, ``calendar_type``, ``edges`` —
+    #:   MOM5
+    #: * ``time_rep`` — CICE5
+    #: * ``um_stash_source``, ``um_version``, ``source`` — UM. ``source`` is a
+    #:   legitimate *global* attribute (CMIP7 Global Attributes, Table 4); this
+    #:   drops it only where UM put it, on a variable.
+    #:
+    #: A denylist rather than an allowlist because the attributes that must
+    #: survive are set all over the pipeline — ``coordinates``,
+    #: ``units_metadata``, ``computed_standard_name``, ``formula_terms``,
+    #: ``bounds`` — and a name missing from an allowlist would silently delete
+    #: mandatory CF metadata. This mirrors the choice already made in
+    #: :meth:`_drop_stale_range_attributes`.
+    _MODEL_NATIVE_VARIABLE_ATTRIBUTES = frozenset(
+        {
+            "cartesian_axis",
+            "calendar_type",
+            "edges",
+            "source",
+            "time_avg_info",
+            "time_rep",
+            "um_stash_source",
+            "um_version",
+        }
+    )
+
+    def _drop_model_native_attributes(self):
+        """Drop ACCESS-native attributes from every variable in the dataset.
+
+        Two things distinguish this from the other attribute cleanups.
+
+        It sweeps *every* variable, not just ``self.cmor_name``. The existing
+        per-variable pops reach only the data variable, which is why
+        ``um_stash_source`` — named for removal in
+        :meth:`Atmosphere_CMORiser.update_attributes` since before this —
+        still reached the published archive on the ``orog`` that model-level
+        files carry as a ``formula_terms`` target, and why ``calendar_type``
+        survived on ``time``.
+
+        It must run **after** :meth:`_check_calendar`. ``calendar_type`` is not
+        inert: MOM writes it instead of the CF ``calendar``, so
+        ``detect_time_frequency_lazy`` and ``_detect_frequency_from_bounds``
+        read it to infer the calendar, and ``_check_calendar`` rewrites a
+        ``GREGORIAN`` value to ``proleptic_gregorian``. Both readers run during
+        ``select_and_process_variables``, well before this, but
+        ``_check_calendar`` runs inside ``update_attributes`` — so this belongs
+        at the end of that method, not at its start.
+        """
+        for name in self.ds.variables:
+            attrs = self.ds[name].attrs
+            for attr in self._MODEL_NATIVE_VARIABLE_ATTRIBUTES:
+                attrs.pop(attr, None)
+
     #: CF Appendix D — the standard name of the quantity each parametric
     #: vertical coordinate computes from its ``formula_terms``. Neither
     #: ``CMIP7_coordinate.json`` nor ``CMIP6_coordinate.json`` carries a

src/access_moppy/ocean.py

@@ -497,6 +497,11 @@ def update_attributes(self):
         # sees the final variable units and the normalized calendar.
         self._apply_units_metadata()
 
+        # Last of all: strip the ACCESS-native attributes the raw files carry.
+        # After _check_calendar, which reads and rewrites calendar_type, and
+        # after every step above that sets attributes of its own.
+        self._drop_model_native_attributes()
+
 
 class Ocean_CMORiser_OM2(Ocean_CMORiser):
     """CMORiser for ocean variables on the ACCESS-OM2 model using B-grid supergrid coordinates."""

src/access_moppy/sea_ice.py

@@ -418,3 +418,8 @@ def update_attributes(self):
         # CF-1.11 units_metadata for the temperature and time units, last so it
         # sees the final variable units and the normalized calendar.
         self._apply_units_metadata()
+
+        # Last of all: strip the ACCESS-native attributes the raw files carry.
+        # After _check_calendar, which reads and rewrites calendar_type, and
+        # after every step above that sets attributes of its own.
+        self._drop_model_native_attributes()

tests/unit/test_variable_attributes.py

@@ -155,3 +155,149 @@ def test_empty_table_values_are_still_skipped(tmp_path):
     attrs = cmoriser.ds["tos"].attrs
     assert "comment" not in attrs
     assert "positive" not in attrs
+
+
+#: What ACCESS writes on its own variables and CMIP does not want. Unlike the
+#: table directives above these arrive with the source file, so the allowlist
+#: applied to the table entry never sees them.
+MODEL_NATIVE = {
+    "time_avg_info": "average_T1,average_T2,average_DT",  # MOM5
+    "time_rep": "averaged",  # CICE5
+    "cartesian_axis": "T",  # MOM5
+    "edges": "st_edges_ocean",  # MOM5
+    "um_stash_source": "m01s00i033",  # UM
+    "um_version": "7.3",  # UM
+    "source": "Unified Model",  # UM
+}
+
+
+def _dataset_with_native_attrs():
+    """A dataset shaped like a model-level file: data variable, coordinate, aux.
+
+    ``orog`` stands in for the ``formula_terms`` target that hybrid-height
+    files carry. It is the case the old per-variable pops could not reach.
+    """
+    ds = xr.Dataset(
+        {
+            "tos": xr.DataArray(
+                np.asarray([1.0, 2.0], dtype=np.float32),
+                dims=["time"],
+                coords={"time": xr.DataArray([0, 1], dims=["time"])},
+            ),
+            "orog": xr.DataArray(np.asarray([0.0], dtype=np.float32), dims=["cell"]),
+        }
+    )
+    ds["tos"].attrs.update(
+        {
+            "standard_name": "sea_surface_temperature",
+            "time_avg_info": MODEL_NATIVE["time_avg_info"],
+        }
+    )
+    ds["time"].attrs.update(
+        {"axis": "T", "cartesian_axis": "T", "calendar_type": "GREGORIAN"}
+    )
+    ds["orog"].attrs.update(
+        {
+            "standard_name": "surface_altitude",
+            "um_stash_source": MODEL_NATIVE["um_stash_source"],
+            "um_version": MODEL_NATIVE["um_version"],
+            "source": MODEL_NATIVE["source"],
+        }
+    )
+    return ds
+
+
+def _cmoriser_with_native_attrs(tmp_path):
+    cmoriser = _cmoriser(tmp_path, CMIP6_ENTRY)
+    cmoriser.ds = _dataset_with_native_attrs()
+    return cmoriser
+
+
+@pytest.mark.unit
+def test_model_native_attributes_are_dropped_from_the_data_variable(tmp_path):
+    """MOM's ``time_avg_info`` reached the published archive on every ocean variable."""
+    cmoriser = _cmoriser_with_native_attrs(tmp_path)
+
+    cmoriser._drop_model_native_attributes()
+
+    assert "time_avg_info" not in cmoriser.ds["tos"].attrs
+
+
+@pytest.mark.unit
+def test_model_native_attributes_are_dropped_from_coordinates(tmp_path):
+    """``calendar_type``/``cartesian_axis`` sit on ``time``, never on the data variable.
+
+    The pops that predate this method are scoped to ``self.cmor_name``, so
+    nothing reached a coordinate.
+    """
+    cmoriser = _cmoriser_with_native_attrs(tmp_path)
+
+    cmoriser._drop_model_native_attributes()
+
+    attrs = cmoriser.ds["time"].attrs
+    assert "cartesian_axis" not in attrs
+    assert "calendar_type" not in attrs
+
+
+@pytest.mark.unit
+def test_model_native_attributes_are_dropped_from_auxiliary_variables(tmp_path):
+    """The ``orog`` a model-level file carries as a ``formula_terms`` target.
+
+    ``um_stash_source`` was already named for removal in the atmosphere
+    CMORiser, yet 919 published files carry it here, because that pop only
+    ever looked at the data variable.
+    """
+    cmoriser = _cmoriser_with_native_attrs(tmp_path)
+
+    cmoriser._drop_model_native_attributes()
+
+    attrs = cmoriser.ds["orog"].attrs
+    for name in ("um_stash_source", "um_version", "source"):
+        assert name not in attrs, f"{name} survived on an auxiliary variable"
+
+
+@pytest.mark.unit
+def test_describing_attributes_survive_the_sweep(tmp_path):
+    """A denylist, so nothing outside it may be touched on any variable."""
+    cmoriser = _cmoriser_with_native_attrs(tmp_path)
+
+    cmoriser._drop_model_native_attributes()
+
+    assert cmoriser.ds["tos"].attrs["standard_name"] == "sea_surface_temperature"
+    assert cmoriser.ds["orog"].attrs["standard_name"] == "surface_altitude"
+    assert cmoriser.ds["time"].attrs["axis"] == "T"
+
+
+@pytest.mark.unit
+def test_calendar_type_is_read_before_it_is_dropped(tmp_path):
+    """Order matters: ``calendar_type`` is an input, not just noise.
+
+    MOM writes it instead of the CF ``calendar``, and ``_check_calendar``
+    rewrites a ``GREGORIAN`` value. Sweeping before that would take the
+    calendar with it, so the sweep belongs at the end of
+    ``update_attributes``.
+    """
+    cmoriser = _cmoriser_with_native_attrs(tmp_path)
+    cmoriser.ds["time"].attrs.update(
+        {"calendar": "GREGORIAN", "units": "days since 0001-01-01"}
+    )
+
+    cmoriser._check_calendar("time")
+    cmoriser._drop_model_native_attributes()
+
+    assert cmoriser.ds["time"].attrs["calendar"] == "proleptic_gregorian"
+    assert "calendar_type" not in cmoriser.ds["time"].attrs
+
+
+@pytest.mark.unit
+def test_global_source_attribute_is_untouched(tmp_path):
+    """``source`` is a legitimate CMIP7 global attribute (Global Attributes, Table 4).
+
+    Only the variable-level ``source`` the UM writes is dropped.
+    """
+    cmoriser = _cmoriser_with_native_attrs(tmp_path)
+    cmoriser.ds.attrs["source"] = "ACCESS-ESM1-6"
+
+    cmoriser._drop_model_native_attributes()
+
+    assert cmoriser.ds.attrs["source"] == "ACCESS-ESM1-6"