Repository: ioos/compliance-checker
Pull request #1159 (2025-04-15 17:59:31)

Domain variables update


Patch:
diff --git a/compliance_checker/cf/cf_1_9.py b/compliance_checker/cf/cf_1_9.py
index 3911c78..0a625f1 100644
--- a/compliance_checker/cf/cf_1_9.py
+++ b/compliance_checker/cf/cf_1_9.py
@@ -8,6 +8,7 @@ from compliance_checker.base import BaseCheck, Result, TestCtx
 from compliance_checker.cf import util
 from compliance_checker.cf.cf_1_8 import CF1_8Check
 from compliance_checker.cf.util import VariableReferenceError, reference_attr_variables
+from compliance_checker.cfutil import get_coordinate_variables
 
 
 class CF1_9Check(CF1_8Check):
@@ -119,176 +120,187 @@ class CF1_9Check(CF1_8Check):
         return prev_return
 
     def check_domain_variables(self, ds: Dataset):
-        # Domain variables should have coordinates attribute, but should have
-        # scalar dimensions
+
         results = []
-        for domain_var in (
-            var
-            for var in ds.get_variables_by_attributes(
-                coordinates=lambda c: c is not None,
+        domain_valid = TestCtx(BaseCheck.MEDIUM, self.section_titles["5.8"])
+        is_ragged_array_repr = cfutil.is_dataset_valid_ragged_array_repr_featureType(
+            ds,
+            getattr(ds, "featureType", ""),
+        )
+        if is_ragged_array_repr:
+            domain_valid.out_of += 1
+            ragged_array_dim_variable, ragged_attr_name = (
+                cfutil.resolve_ragged_array_dimension(ds)
             )
-        ):
-
-            # IMPLICIT CONFORMANCE REQUIRED 1/4
-            # Has a dimensions *NetCDF* attribute
-            try:
-                dim_nc_attr = domain_var.getncattr("dimensions")
-            # most variables are unlikely to be domain variables, so don't treat this
-            # as a failure
-            except AttributeError:
-                continue
-            # IMPLICIT CONFORMANCE REQUIRED 2/4
-            # Aforementioned dimensions attribute is comprised of space separated
-            # dimension names which must exist in the file
-            domain_valid = TestCtx(BaseCheck.MEDIUM, self.section_titles["5.8"])
-            domain_valid.out_of += 2
-            domain_dims, dim_errors = reference_attr_variables(ds, dim_nc_attr, " ")
-            if dim_errors:
-                errors_str = ", ".join(dim_errors)
-                domain_valid.messages.append(
-                    "Could not find the following "
-                    "dimensions referenced in "
-                    "dimensions attribute from "
-                    "domain variable "
-                    f"{domain_var.name}: {errors_str}",
-                )
-            else:
-                domain_valid.score += 1
-            domain_coord_vars, domain_coord_var_errors = reference_attr_variables(
+            dim_name = getattr(ragged_array_dim_variable, ragged_attr_name)
+            referenced_dim = reference_attr_variables(
                 ds,
-                domain_var.coordinates,
-                " ",
+                dim_name,
+                reference_type="dimensions",
             )
-            if domain_coord_var_errors:
-                errors_str = ", ".join(err.name for err in domain_coord_var_errors)
+            if isinstance(referenced_dim, VariableReferenceError):
                 domain_valid.messages.append(
-                    "Could not find the following "
-                    "variables referenced in "
-                    "coordinates attribute from "
-                    "domain variable "
-                    f"{domain_var.name}: {errors_str}",
+                    f"Found ragged array variable {ragged_array_dim_variable.name}, "
+                    f"but dimension {dim_name} referenced from {ragged_attr_name} does not exist in file",
                 )
-            else:
-                domain_valid.score += 1
 
-            is_ragged_array_repr = (
-                cfutil.is_dataset_valid_ragged_array_repr_featureType(
-                    ds,
-                    getattr(ds, "featureType", ""),
-                )
-            )
-            if is_ragged_array_repr:
-                domain_valid.out_of += 1
-                ragged_array_dim_variable, ragged_attr_name = (
-                    cfutil.resolve_ragged_array_dimension(ds)
-                )
-                dim_name = getattr(ragged_array_dim_variable, ragged_attr_name)
-                referenced_dim = reference_attr_variables(
-                    ds,
-                    dim_name,
-                    reference_type="dimension",
-                )
-                if isinstance(referenced_dim, VariableReferenceError):
+            coord_var_reference_failures = []
+            for coord_var in reference_attr_variables(ds, dim_name, " ", "dimensions"):
+                if isinstance(coord_var, VariableReferenceError):
+                    coord_var_reference_failures.append(coord_var)
+                    domain_valid.messages.append(
+                        f"Referenced coordinate variable {coord_var} does not exist in file",
+                    )
+                    continue
+                # TODO: check for label variables
+                if not set(
+                    util.get_possible_label_variable_dimensions(coord_var),
+                ).issubset({referenced_dim}):
                     domain_valid.messages.append(
                         f"Found ragged array variable {ragged_array_dim_variable.name}, "
                         f"but dimension {dim_name} referenced from {ragged_attr_name} does not exist in file",
                     )
+                else:
+                    domain_valid.score += 1
+        else:
+            # IMPLICIT CONFORMANCE REQUIRED 1/4
+            # Domain variables must have a dimensions attribute
+            for domain_var in (
+                ds.variables[var]
+                for var in ds.variables
+                if "dimensions" in ds.variables[var].ncattrs()
+            ):
+                # store NetCDF variable attribute named "dimensions"
+                dim_nc_attr = domain_var.getncattr("dimensions")
+                # Every variable should have dimensionality (not to be confused with
+                # dimensions NetCDF attribute here) even if scalar/empty, but non-empty
+                # dimensions aren't allowed for domain variables
 
-                coord_var_reference_failures = []
-                for coord_var in reference_attr_variables(ds, dim_name, " "):
-                    if isinstance(coord_var, VariableReferenceError):
-                        coord_var_reference_failures.append(coord_var)
-                        domain_valid.messages.append(
-                            f"Referenced coordinate variable {coord_var} does not exist in file",
-                        )
-                        continue
-                    # TODO: check for label variables
-                    if not set(
-                        util.get_possible_label_variable_dimensions(coord_var),
-                    ).issubset({referenced_dim}):
-                        domain_valid.messages.append(
-                            f"Found ragged array variable {ragged_array_dim_variable.name}, "
-                            f"but dimension {dim_name} referenced from {ragged_attr_name} does not exist in file",
-                        )
-                    else:
-                        domain_valid.score += 1
-            else:
-                for coord_var in domain_coord_vars:
-                    domain_valid.out_of += 1
-                    domain_dims_names = {var.name for var in domain_dims}
-                    variable_dim = util.get_possible_label_variable_dimensions(
-                        coord_var,
+                domain_valid.out_of += 3
+                if domain_var.dimensions:
+                    domain_valid.messages.append(
+                        "Domain variable "
+                        f"{domain_var.name} should not have non-scalar/"
+                        "non-empty variable dimensions",
                     )
-                    if not (
-                        set(
-                            util.get_possible_label_variable_dimensions(coord_var),
-                        ).issubset(domain_dims_names)
-                    ):
+                else:
+                    domain_valid.score += 1
+
+                # IMPLICIT CONFORMANCE REQUIRED 2/4
+                # Aforementioned dimensions attribute is comprised of space separated
+                # dimension names which must exist in the file
+
+                domain_dims, dim_errors = reference_attr_variables(
+                    ds,
+                    dim_nc_attr,
+                    " ",
+                    "dimensions",
+                )
+                if dim_errors:
+                    errors_str = ", ".join(dim_errors)
+                    domain_valid.messages.append(
+                        "Could not find the following "
+                        "dimensions referenced in "
+                        "dimensions attribute from "
+                        "domain variable "
+                        f"{domain_var.name}: {errors_str}",
+                    )
+                else:
+                    domain_valid.score += 1
+                if "coordinates" in domain_var.ncattrs():
+                    domain_coord_vars, domain_coord_var_errors = (
+                        reference_attr_variables(ds, domain_var.coordinates, " ")
+                    )
+                    if domain_coord_var_errors:
+                        errors_str = ", ".join(
+                            err.name for err in domain_coord_var_errors
+                        )
                         domain_valid.messages.append(
                             "Could not find the following "
                             "variables referenced in "
                             "coordinates attribute from "
                             "domain variable "
-                            f"{variable_dim}: {domain_dims_names}",
+                            f"{domain_var.name}: {errors_str}",
                         )
                     else:
                         domain_valid.score += 1
+                        for coord_var in domain_coord_vars:
+                            if coord_var not in get_coordinate_variables(ds):
+                                continue
+                            domain_valid.out_of += 1
+                            domain_dims_names = {var.name for var in domain_dims}
+                            variable_dim = util.get_possible_label_variable_dimensions(
+                                coord_var,
+                            )
+                            if not (
+                                set(
+                                    util.get_possible_label_variable_dimensions(
+                                        coord_var,
+                                    ),
+                                ).issubset(domain_dims_names)
+                            ):
+                                domain_valid.messages.append(
+                                    "Could not find the following "
+                                    "variables referenced in "
+                                    "coordinates attribute from "
+                                    "domain variable "
+                                    f"{variable_dim}: {domain_dims_names}",
+                                )
+                            else:
+                                domain_valid.score += 1
 
-            # not in conformance docs, but mentioned as recommended anyways
-            domain_valid.out_of += 1
-            long_name = getattr(domain_var, "long_name", None)
-            if long_name is None or not isinstance(long_name, str):
-                domain_valid.messages.append(
-                    f"For domain variable {domain_var.name} "
-                    f"it is recommended that attribute long_name be present and a string",
-                )
-                results.append(domain_valid.to_result())
-            else:
-                domain_valid.score += 1
-            appendix_a_not_recommended_attrs = []
-            for attr_name in domain_var.ncattrs():
-                if (
-                    attr_name in self.appendix_a
-                    and "D" not in self.appendix_a[attr_name]["attr_loc"]
-                ):
-                    appendix_a_not_recommended_attrs.append(attr_name)
+                # not in conformance docs, but mentioned as recommended anyways
+                domain_valid.out_of += 1
+                long_name = getattr(domain_var, "long_name", None)
+                if long_name is None or not isinstance(long_name, str):
+                    domain_valid.messages.append(
+                        f"For domain variable {domain_var.name} "
+                        f"it is recommended that attribute long_name be present and a string",
+                    )
+                else:
+                    domain_valid.score += 1
 
-            domain_valid.out_of += 1
-            if appendix_a_not_recommended_attrs:
-                domain_valid.messages.append(
-                    f"The following attributes appear in variable {domain_var.name} "
-                    "and CF Appendix A, but are not for use in domain variables: "
-                    f"{appendix_a_not_recommended_attrs}",
-                )
-            else:
-                # no errors occurred
-                domain_valid.score += 1
+                appendix_a_not_recommended_attrs = []
+                for attr_name in domain_var.ncattrs():
+                    if (
+                        attr_name in self.appendix_a
+                        and "D" not in self.appendix_a[attr_name]["attr_loc"]
+                    ):
+                        appendix_a_not_recommended_attrs.append(attr_name)
 
-            # IMPLEMENTATION CONFORMANCE 5.8 REQUIRED 4/4
-            # variables named by domain variable's cell_measures attributes must themselves be a subset
-            # of dimensions named by domain variable's dimensions NetCDF attribute
-            if hasattr(domain_var, "cell_measures"):
-                cell_measures_var_names = regex.findall(
-                    r"\b(?:area|volume):\s+(\w+)",
-                    domain_var.cell_measures,
-                )
-                # check exist
-                for var_name in cell_measures_var_names:
-                    try:
-                        cell_measures_variable = ds.variables[var_name]
-                    except ValueError:
-                        # TODO: what to do here?
-                        continue
-                    domain_coord_var_names = {
-                        var_like.name for var_like in domain_coord_vars
-                    }
-                    domain_valid.assert_true(
-                        set(cell_measures_variable.dimensions).issubset(
-                            domain_coord_var_names,
-                        ),
-                        "Variables named in the cell_measures attributes must have a dimensions attribute with "
-                        "values that are a subset of the referring domain variable's dimension attribute",
+                domain_valid.out_of += 1
+                if appendix_a_not_recommended_attrs:
+                    domain_valid.messages.append(
+                        f"The following attributes appear in variable {domain_var.name} "
+                        "and CF Appendix A, but are not for use in domain variables: "
+                        f"{appendix_a_not_recommended_attrs}",
                     )
+                else:
+                    # no errors occurred
+                    domain_valid.score += 1
+
+                # IMPLEMENTATION CONFORMANCE 5.8 REQUIRED 4/4
+                # variables named by domain variable's cell_measures attributes must themselves be a subset
+                # of dimensions named by domain variable's dimensions NetCDF attribute
+                if hasattr(domain_var, "cell_measures"):
+                    cell_measures_var_names = regex.findall(
+                        r"\b(?:area|volume):\s+(\w+)",
+                        domain_var.cell_measures,
+                    )
+                    # check exist
+                    for var_name in cell_measures_var_names:
+                        try:
+                            cell_measures_variable = ds.variables[var_name]
+                        except ValueError:
+                            # TODO: what to do here?
+                            continue
+                        domain_valid.assert_true(
+                            set(cell_measures_variable.dimensions).issubset(
+                                {dd.name for dd in domain_dims},
+                            ),
+                            "Variables named in the cell_measures attributes must have dimensions with "
+                            "values that are a subset of the referring domain variable's dimensions attribute",
+                        )
 
             results.append(domain_valid.to_result())
 
diff --git a/compliance_checker/cf/util.py b/compliance_checker/cf/util.py
index f2e4a35..b6e711f 100644
--- a/compliance_checker/cf/util.py
+++ b/compliance_checker/cf/util.py
@@ -466,7 +466,7 @@ def reference_attr_variables(
     dataset: Dataset,
     attributes_string: str,
     split_by: str = None,
-    reference_type: str = "variable",
+    reference_type: str = "variables",
     group: Union[Group, Dataset] = None,
 ):
     """
@@ -476,21 +476,23 @@ def reference_attr_variables(
     references, errors = [], []
     if attributes_string is None:
         return None, None
-    elif reference_type == "variable":
-        if split_by is None:
-            return_val = dataset.variables.get(
-                attributes_string,
-                VariableReferenceError(attributes_string),
-            )
-            if not isinstance(return_val, VariableReferenceError):
-                return return_val, None
-            else:
-                return None, return_val
+    if reference_type not in {"variables", "dimensions"}:
+        raise ValueError("reference_type must be one of 'variables' or 'dimensions'")
+    reference_attr = getattr(dataset, reference_type)
+    if split_by is None:
+        return_val = reference_attr.get(
+            attributes_string,
+            VariableReferenceError(attributes_string),
+        )
+        if not isinstance(return_val, VariableReferenceError):
+            return return_val, None
         else:
-            string_proc = attributes_string.split(split_by)
-            for var_name in string_proc:
-                if var_name in dataset.variables:
-                    references.append(dataset.variables[var_name])
-                else:
-                    errors.append(VariableReferenceError(var_name))
-            return references, errors
+            return None, return_val
+    else:
+        string_proc = attributes_string.split(split_by)
+        for var_name in string_proc:
+            if var_name in reference_attr:
+                references.append(reference_attr[var_name])
+            else:
+                errors.append(VariableReferenceError(var_name))
+        return references, errors
diff --git a/docs/cf_feature_parity_matrix.md b/docs/cf_feature_parity_matrix.md
index 08d1761..7dfce1a 100644
--- a/docs/cf_feature_parity_matrix.md
+++ b/docs/cf_feature_parity_matrix.md
@@ -43,7 +43,7 @@ Most CF features are supported, but several are either partially implemented or
 | 5.6. Horizontal Coordinate Reference Systems, Grid Mappings, and Projections | partial | Needs implementation of a few more sections | https://github.com/ioos/compliance-checker/issues/950
 | 5.6.1. Use of the CRS Well-known Text Format | yes |
 | 5.7. Scalar Coordinate Variables | yes |
-| 5.8. Domain Variables | partial | Needs ragged array support | https://github.com/ioos/compliance-checker/issues/951
+| 5.8. Domain Variables | yes |
 | 6. Labels and Alternative Coordinates | yes
 | 6.1. Labels | yes
 | 6.1.1. Geographic Regions | partial | Implicit from handling of standard names - does not check region names


Test patch:
diff --git a/compliance_checker/tests/data/indexed_ragged_domain.cdl b/compliance_checker/tests/data/indexed_ragged_domain.cdl
new file mode 100644
index 0000000..a8f4512
--- /dev/null
+++ b/compliance_checker/tests/data/indexed_ragged_domain.cdl
@@ -0,0 +1,41 @@
+netcdf indexed_ragged_domain {
+dimensions:
+  station = 23 ;
+  obs = UNLIMITED ;
+  name_strlen = 23 ;
+
+variables:
+  char domain ;
+    domain:dimensions = "obs" ;
+    domain:coordinates = "time lat lon alt station_name" ;
+    domain:long_name = "Domain with a discrete sampling geometry" ;
+  float lon(station) ;
+    lon:standard_name = "longitude" ;
+    lon:long_name = "station longitude" ;
+    lon:units = "degrees_east" ;
+  float lat(station) ;
+    lat:standard_name = "latitude" ;
+    lat:long_name = "station latitude" ;
+    lat:units = "degrees_north" ;
+  float alt(station) ;
+    alt:long_name = "vertical distance above the surface" ;
+    alt:standard_name = "height" ;
+    alt:units = "m" ;
+    alt:positive = "up" ;
+    alt:axis = "Z" ;
+  char station_name(station, name_strlen) ;
+    station_name:long_name = "station name" ;
+    station_name:cf_role = "timeseries_id" ;
+  int station_info(station) ;
+    station_info:long_name = "some kind of station info" ;
+  int stationIndex(obs) ;
+    stationIndex:long_name = "which station this obs is for" ;
+    stationIndex:instance_dimension = "station" ;
+  double time(obs) ;
+    time:standard_name = "time" ;
+    time:long_name = "time of measurement" ;
+    time:units = "days since 1970-01-01 00:00:00" ;
+
+// global attributes:
+    :featureType = "timeSeries" ;
+}
diff --git a/compliance_checker/tests/resources.py b/compliance_checker/tests/resources.py
index 8b48915..89c8730 100644
--- a/compliance_checker/tests/resources.py
+++ b/compliance_checker/tests/resources.py
@@ -74,6 +74,7 @@ STATIC_FILES = {
     "illegal-aux-coords": get_filename("tests/data/illegal-aux-coords.cdl"),
     "index_ragged": get_filename("tests/data/index_ragged.cdl"),
     "index_ragged2": get_filename("tests/data/index_ragged2.cdl"),
+    "indexed_ragged_domain": get_filename("tests/data/indexed_ragged_domain.cdl"),
     "ints64": get_filename("tests/data/ints64.cdl"),
     "ioos_gold_1_1": get_filename("tests/data/ioos_1_1.cdl"),
     "kibesillah": get_filename("tests/data/examples/kibesillah.cdl"),
diff --git a/compliance_checker/tests/test_cf.py b/compliance_checker/tests/test_cf.py
index bc08d9a..f3ebe5b 100644
--- a/compliance_checker/tests/test_cf.py
+++ b/compliance_checker/tests/test_cf.py
@@ -10,7 +10,7 @@ from itertools import chain
 import numpy as np
 import pytest
 import requests_mock
-from netCDF4 import stringtoarr
+from netCDF4 import Dataset, stringtoarr
 
 from compliance_checker import cfutil
 from compliance_checker.cf.appendix_d import no_missing_terms
@@ -3210,27 +3210,32 @@ class TestCF1_9(BaseTestCase):
         # OK, coordinates in cell_measures are subset of coordinates of
         # referring domain variable's coordinates attribute
         results = self.cf.check_domain_variables(dataset)
-        # "time" dimension named in domain variable not in dataset dimensions
-        assert results[0].msgs
+        assert not results[0].msgs
         # failing example, coordinates for cell_measures variable are no longer subset
+        # "time" dimension named in domain variable not in dataset dimensions
         domain_var.cell_measures = "volume: cube_bad"
         dataset.createVariable("cube_bad", "f8", ("lon", "lat", "depth", "time"))
         results = self.cf.check_domain_variables(dataset)
         assert (
-            "Variables named in the cell_measures attributes must "
-            "have a dimensions attribute with values that are a "
-            "subset of the referring domain variable's dimension "
-            "attribute" in results[0].msgs
+            "Variables named in the cell_measures attributes must have "
+            "dimensions with values that are a subset of the referring domain "
+            "variable's dimensions attribute" in results[0].msgs
         )
         del dataset
         dataset = MockTimeSeries()
-        # domain should be dimensionless -- currently not an error in
-        # compliance checker, but not detected as a domain variable either
+        # domain should be dimensionless in terms of variable dimensions
         domain_var = dataset.createVariable("domain", "c", ("time",))
+        domain_var.setncattr("dimensions", "time")
         domain_var.long_name = "Domain variable"
-        domain_var.coordinates = "lon lat depth"
         results = self.cf.check_domain_variables(dataset)
-        assert len(results) == 0
+        assert (
+            "Domain variable domain should not have non-scalar/"
+            "non-empty variable dimensions" in results[0].msgs
+        )
+        # check ragged array results
+        dataset = Dataset(STATIC_FILES["indexed_ragged_domain"])
+        results = self.cf.check_domain_variables(dataset)
+        assert results[0].value[0] == results[0].value[1]
 
 
 class TestCFUtil(BaseTestCase):