fix atmophere and ocean variables workflow geophysical variable fails

src/access_moppy/atmosphere.py

@@ -369,6 +369,17 @@ def update_attributes(self):
         self.ds[self.cmor_name].attrs.update(
             {k: v for k, v in cmor_attrs.items() if v not in (None, "")}
         )
+
+        # A geophysical data variable must never carry an `axis` attribute: the
+        # WCRP "Geophysical Variable Detection" check classifies any variable
+        # with `axis` as a coordinate and excludes it (mrsos, derived from a
+        # soil-layer field, otherwise inherits axis='Z'/positive='down' from the
+        # source vertical coordinate). Drop `axis` unconditionally, and `positive`
+        # unless the CMOR table actually declares one for this variable.
+        self.ds[self.cmor_name].attrs.pop("axis", None)
+        if not cmor_attrs.get("positive"):
+            self.ds[self.cmor_name].attrs.pop("positive", None)
+
         var_type = cmor_attrs.get("type", "double")
         self.ds[self.cmor_name] = self.ds[self.cmor_name].astype(
             self.type_mapping.get(var_type, np.float64)

src/access_moppy/ocean.py

@@ -267,6 +267,14 @@ def update_attributes(self):
             self.ds["vertices_longitude"].attrs.update(
                 {"standard_name": "longitude", "units": "degrees_east"}
             )
+
+            # Point the data variable at the curvilinear auxiliary coordinates we
+            # just built. The model file's `coordinates` attribute (e.g.
+            # "geolon_t geolat_t") names the native grid variables, which are not
+            # carried into the CMORised output — leaving it stale makes the WCRP
+            # ATTR004 "coordinates as-variable" check fail on missing references.
+            # (self.cmor_name is guaranteed present: its dims were read above.)
+            self.ds[self.cmor_name].attrs["coordinates"] = "latitude longitude"
         else:
             # Drop dimensions that are no longer referenced by any data variable.
             used_dims = set()

tests/unit/test_atmosphere.py

@@ -1851,6 +1851,152 @@ def test_unsupported_calc_type_lists_supported(self, tmp_path):
         assert "internal" in msg
 
 
+# ---------------------------------------------------------------------------
+# Tests for update_attributes() axis/positive stripping on the data variable
+# ---------------------------------------------------------------------------
+
+
+def _make_axis_strip_cmoriser(cmor_name, var_attrs, vocab_variable):
+    """
+    Build a minimal Atmosphere_CMORiser to exercise the axis/positive cleanup
+    in update_attributes(). The main variable is a (time, lat, lon) field whose
+    attrs may contain stray axis/positive leaked from a source vertical coord.
+    """
+    cmoriser = object.__new__(Atmosphere_CMORiser)
+    cmoriser.cmor_name = cmor_name
+    cmoriser.type_mapping = CMORiser.type_mapping
+
+    ds = xr.Dataset(
+        {
+            cmor_name: xr.DataArray(
+                np.ones((1, 2, 2), dtype=np.float32),
+                dims=["time", "lat", "lon"],
+                coords={
+                    "time": (
+                        ["time"],
+                        np.array([0.0]),
+                        {"units": "days since 1850-01-01", "calendar": "standard"},
+                    ),
+                    "lat": [0.0, 1.0],
+                    "lon": [0.0, 1.0],
+                },
+                attrs=var_attrs,
+            )
+        }
+    )
+    cmoriser.ds = ds
+
+    vocab = MagicMock()
+    vocab.get_required_global_attributes.return_value = {}
+    vocab.axes = {
+        "time": {
+            "out_name": "time",
+            "standard_name": "time",
+            "units": "days since 1850-01-01",
+            "type": "double",
+            "axis": "T",
+        },
+        "lat": {"out_name": "lat"},
+        "lon": {"out_name": "lon"},
+    }
+    vocab.variable = vocab_variable
+    cmoriser.vocab = vocab
+    cmoriser._check_units = MagicMock()
+    cmoriser._check_calendar = MagicMock()
+    cmoriser._check_range = MagicMock()
+    return cmoriser
+
+
+class TestUpdateAttributesAxisPositiveStripping:
+    """
+    A geophysical (time, lat, lon) data variable must not carry an `axis`
+    attribute — the WCRP "Geophysical Variable Detection" check excludes any
+    variable with `axis`, so mrsos (derived from a soil-layer field that leaks
+    axis='Z'/positive='down') was wrongly classified as a coordinate.
+    """
+
+    @pytest.mark.unit
+    def test_axis_stripped_from_data_variable(self):
+        """A leaked axis='Z' is removed from the main variable."""
+        cmoriser = _make_axis_strip_cmoriser(
+            "mrsos",
+            var_attrs={
+                "standard_name": "mass_content_of_water_in_soil_layer",
+                "units": "kg m-2",
+                "axis": "Z",
+            },
+            vocab_variable={
+                "standard_name": "mass_content_of_water_in_soil_layer",
+                "units": "kg m-2",
+                "type": "real",
+            },
+        )
+        cmoriser.update_attributes()
+
+        assert "axis" not in cmoriser.ds["mrsos"].attrs
+
+    @pytest.mark.unit
+    def test_undeclared_positive_stripped(self):
+        """positive='down' is removed when the CMOR table declares no positive."""
+        cmoriser = _make_axis_strip_cmoriser(
+            "mrsos",
+            var_attrs={
+                "standard_name": "mass_content_of_water_in_soil_layer",
+                "units": "kg m-2",
+                "axis": "Z",
+                "positive": "down",
+            },
+            vocab_variable={
+                "standard_name": "mass_content_of_water_in_soil_layer",
+                "units": "kg m-2",
+                "type": "real",
+                "positive": "",  # CMOR mrsos entry carries an empty positive
+            },
+        )
+        cmoriser.update_attributes()
+
+        assert "positive" not in cmoriser.ds["mrsos"].attrs
+
+    @pytest.mark.unit
+    def test_declared_positive_preserved(self):
+        """A flux variable whose CMOR table declares positive keeps it."""
+        cmoriser = _make_axis_strip_cmoriser(
+            "hfls",
+            var_attrs={
+                "standard_name": "surface_upward_latent_heat_flux",
+                "units": "W m-2",
+                "positive": "down",  # stale/leaked value
+            },
+            vocab_variable={
+                "standard_name": "surface_upward_latent_heat_flux",
+                "units": "W m-2",
+                "type": "real",
+                "positive": "up",  # authoritative CMOR value
+            },
+        )
+        cmoriser.update_attributes()
+
+        assert cmoriser.ds["hfls"].attrs.get("positive") == "up"
+
+    @pytest.mark.unit
+    def test_normal_variable_without_axis_unaffected(self):
+        """A regular variable (no axis/positive) passes through unchanged."""
+        cmoriser = _make_axis_strip_cmoriser(
+            "tas",
+            var_attrs={"standard_name": "air_temperature", "units": "K"},
+            vocab_variable={
+                "standard_name": "air_temperature",
+                "units": "K",
+                "type": "real",
+            },
+        )
+        cmoriser.update_attributes()
+
+        assert "axis" not in cmoriser.ds["tas"].attrs
+        assert "positive" not in cmoriser.ds["tas"].attrs
+        assert cmoriser.ds["tas"].attrs.get("standard_name") == "air_temperature"
+
+
 # ---------------------------------------------------------------------------
 # Tests for _retarget_renamed_references (hybrid-height coordinate/formula_terms)
 # ---------------------------------------------------------------------------

tests/unit/test_ocean.py

@@ -854,6 +854,51 @@ def test_spatial_variable_grid_coords_still_added(
         for var in ("latitude", "longitude", "vertices_latitude", "vertices_longitude"):
             assert var in cmoriser.ds, f"'{var}' missing for spatial variable"
 
+    @pytest.mark.unit
+    def test_spatial_variable_coordinates_point_to_lat_lon(
+        self, mock_vocab, spatial_mapping, temp_dir
+    ):
+        """Data variable's coordinates attr must reference the supergrid lat/lon.
+
+        The WCRP ATTR004 'coordinates as-variable' check fails when the attribute
+        names variables not present in the file.
+        """
+        cmoriser = _make_cmoriser(
+            mock_vocab, spatial_mapping, "Omon.tos", temp_dir, _spatial_ds()
+        )
+        with patch.object(cmoriser, "_check_calendar"):
+            cmoriser.update_attributes()
+
+        assert cmoriser.ds["tos"].attrs.get("coordinates") == "latitude longitude"
+
+    @pytest.mark.unit
+    def test_stale_model_coordinates_overwritten(
+        self, mock_vocab, spatial_mapping, temp_dir
+    ):
+        """A stale 'geolon_t geolat_t' inherited from the model file is replaced."""
+        ds = _spatial_ds()
+        ds["tos"].attrs["coordinates"] = "geolon_t geolat_t"
+        cmoriser = _make_cmoriser(mock_vocab, spatial_mapping, "Omon.tos", temp_dir, ds)
+        with patch.object(cmoriser, "_check_calendar"):
+            cmoriser.update_attributes()
+
+        coords_attr = cmoriser.ds["tos"].attrs.get("coordinates")
+        assert coords_attr == "latitude longitude"
+        assert "geolon_t" not in coords_attr
+
+    @pytest.mark.unit
+    def test_scalar_variable_coordinates_not_set(
+        self, mock_vocab, scalar_mapping, temp_dir
+    ):
+        """A scalar variable (no i/j) must not gain a curvilinear coordinates attr."""
+        cmoriser = _make_cmoriser(
+            mock_vocab, scalar_mapping, "Omon.zostoga", temp_dir, _scalar_ds()
+        )
+        with patch.object(cmoriser, "_check_calendar"):
+            cmoriser.update_attributes()
+
+        assert cmoriser.ds["zostoga"].attrs.get("coordinates") != "latitude longitude"
+
     @pytest.mark.unit
     def test_time_coordinate_gets_cf_attributes(
         self, mock_vocab, spatial_mapping, temp_dir