Merge pull request #1159 from benjwadams/domain_variables_update

Domain variables update

Author: benjwadams

compliance_checker/cf/cf_1_9.py

@@ -8,6 +8,7 @@
 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 @@ def check_time_coordinate(self, ds):
         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())
 

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