Repository: ioos/compliance-checker
Pull request #1140 (2025-02-27 20:47:09)

Exclude projected x/y from treatment as lon/lat
Addresses #1064


Patch:
diff --git a/compliance_checker/cf/cf_1_7.py b/compliance_checker/cf/cf_1_7.py
index 7e6cb03..c2c1884 100644
--- a/compliance_checker/cf/cf_1_7.py
+++ b/compliance_checker/cf/cf_1_7.py
@@ -75,6 +75,7 @@ class CF1_7Check(CF1_6Check):
         external_vars_ctx = TestCtx(BaseCheck.MEDIUM, self.section_titles["2.6.3"])
         # IMPLEMENTATION CONFORMANCE 2.6.3 REQUIRED 2/2
         try:
+            # Should external variables be stored in dataset state?
             external_var_names = set(ds.external_variables.strip().split())
 
             bad_external_var_names = external_var_names.intersection(ds.variables)
@@ -541,17 +542,6 @@ class CF1_7Check(CF1_6Check):
         else:
             return (True, msg)
 
-    def _get_projdb_conn(self):
-        """
-        Return a SQLite Connection to the PROJ database.
-
-        Returns:
-            sqlite3.Connection
-        """
-
-        proj_db_path = os.path.join(pyproj.datadir.get_data_dir(), "proj.db")
-        return sqlite3.connect(proj_db_path)
-
     def _exec_query_str_with_params(self, qstr, argtuple):
         """
         Execute a query string in a database connection with the given argument
@@ -562,8 +552,9 @@ class CF1_7Check(CF1_6Check):
         :rtype set
         """
 
-        conn = self._get_projdb_conn()
-        return conn.execute(qstr, argtuple)
+        proj_db_path = os.path.join(pyproj.datadir.get_data_dir(), "proj.db")
+        with sqlite3.connect(proj_db_path) as conn:
+            return conn.execute(qstr, argtuple)
 
     def _evaluate_geographic_crs_name(self, val):
         """
diff --git a/compliance_checker/cf/cf_1_8.py b/compliance_checker/cf/cf_1_8.py
index 8f262db..4938343 100644
--- a/compliance_checker/cf/cf_1_8.py
+++ b/compliance_checker/cf/cf_1_8.py
@@ -545,7 +545,9 @@ class LineGeometry(GeometryStorage):
             )
         # if a multipart
         if self.node_count is not None:
-            same_length = len(self.coord_vars[0]) == self.node_count[:].sum()
+            same_length = (
+                len(self.coord_vars[0]) == np.atleast_1d(self.node_count)[:].sum()
+            )
             if not same_length:
                 geom_errors.append(
                     "Coordinate variables must be the same "
diff --git a/compliance_checker/cf/cf_1_9.py b/compliance_checker/cf/cf_1_9.py
index 404e335..f63cf0d 100644
--- a/compliance_checker/cf/cf_1_9.py
+++ b/compliance_checker/cf/cf_1_9.py
@@ -114,14 +114,11 @@ class CF1_9Check(CF1_8Check):
                 coordinates=lambda c: c is not None,
             )
         ):
-            # all variables have dimensions attribute, but should be scalar
-            if not ds.dimensions == ():
-                continue
 
             # IMPLICIT CONFORMANCE REQUIRED 1/4
             # Has a dimensions *NetCDF* attribute
             try:
-                dim_nc_attr = var.getncattr(dimensions)
+                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:
@@ -131,9 +128,7 @@ class CF1_9Check(CF1_8Check):
             # 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, domain_var.getncattr("dimensions"), " ", "dimensions",
-            )
+            domain_dims, dim_errors = reference_attr_variables(ds, dim_nc_attr, " ")
             if dim_errors:
                 errors_str = ", ".join(dim_errors)
                 domain_valid.messages.append(
@@ -146,10 +141,12 @@ class CF1_9Check(CF1_8Check):
             else:
                 domain_valid.score += 1
             domain_coord_vars, domain_coord_var_errors = reference_attr_variables(
-                ds, domain_var.coordinates, " ",
+                ds,
+                domain_var.coordinates,
+                " ",
             )
             if domain_coord_var_errors:
-                errors_str = ", ".join(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 "
@@ -160,56 +157,71 @@ class CF1_9Check(CF1_8Check):
             else:
                 domain_valid.score += 1
 
-            coord_var_dim_failures = []
-            is_ragged_array_repr = is_dataset_valid_ragged_array_repr_featureType(
-                ds, getattr(ds, "featureType", ""),
+            is_ragged_array_repr = (
+                cfutil.is_dataset_valid_ragged_array_repr_featureType(
+                    ds,
+                    getattr(ds, "featureType", ""),
+                )
             )
             if is_ragged_array_repr:
-                for var in domain_coord_vars:
-                    domain_valid.out_of += 1
-                    ragged_array_dim_variable, ragged_attr_name = (
-                        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",
+                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):
+                    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",
                     )
-                    if isinstance(referenced_dim, VariableReferenceError):
+
+                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 {attr_name} does not exist in file",
+                            f"but dimension {dim_name} referenced from {ragged_attr_name} does not exist in file",
                         )
-
-                    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 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 {attr_name} does not exist in file",
-                            )
-                        else:
-                            domain_valid.scored += 1
-                    pass
+                    else:
+                        domain_valid.score += 1
             else:
-                for var in domain_coord_vars:
+                for coord_var in domain_coord_vars:
                     domain_valid.out_of += 1
-                    if not util.get_possible_label_variable_dimensions(
+                    domain_dims_names = {var.name for var in domain_dims}
+                    variable_dim = util.get_possible_label_variable_dimensions(
                         coord_var,
-                    ).issubset(domain_dims):
-                        domain_valid.messages.append("Domain dimension failure")
+                    )
+                    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.scored += 1
+                        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(
@@ -217,7 +229,8 @@ class CF1_9Check(CF1_8Check):
                     f"it is recommended that attribute long_name be present and a string",
                 )
                 results.append(domain_valid.to_result())
-                continue
+            else:
+                domain_valid.score += 1
             appendix_a_not_recommended_attrs = []
             for attr_name in domain_var.ncattrs():
                 if (
@@ -226,15 +239,16 @@ class CF1_9Check(CF1_8Check):
                 ):
                     appendix_a_not_recommended_attrs.append(attr_name)
 
+            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}",
                 )
-
-            # no errors occurred
-            domain_valid.score += 1
+            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
diff --git a/compliance_checker/cf/util.py b/compliance_checker/cf/util.py
index 823c10a..f2e4a35 100644
--- a/compliance_checker/cf/util.py
+++ b/compliance_checker/cf/util.py
@@ -413,7 +413,7 @@ def get_possible_label_variable_dimensions(variable: Variable) -> tuple[int, ...
     Return dimensions if non-char variable, or return variable dimensions
     without trailing dimension if char variable, treating it as a label variable.
     """
-    if variable.kind == "C" and len(variable.dimensions) > 0:
+    if variable.dtype.kind == "C" and len(variable.dimensions) > 0:
         return variable.dimensions[:-1]
     return variable.dimensions
 
@@ -475,13 +475,17 @@ def reference_attr_variables(
     """
     references, errors = [], []
     if attributes_string is None:
-        return None
+        return None, None
     elif reference_type == "variable":
         if split_by is None:
-            return dataset.variables.get(
+            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
         else:
             string_proc = attributes_string.split(split_by)
             for var_name in string_proc:
diff --git a/compliance_checker/cfutil.py b/compliance_checker/cfutil.py
index e0385dd..f94c479 100644
--- a/compliance_checker/cfutil.py
+++ b/compliance_checker/cfutil.py
@@ -494,6 +494,15 @@ def get_latitude_variables(nc):
     for variable in nc.get_variables_by_attributes(standard_name="latitude"):
         latitude_variables.append(variable.name)
 
+    # Then axis
+    for variable in nc.get_variables_by_attributes(axis="Y"):
+        if not (
+            variable.name in latitude_variables
+            or getattr(variable, "standard_name", None)
+            in {"projection_y_coordinate", "projection_y_angular_coordinate"}
+        ):
+            latitude_variables.append(variable.name)
+
     check_fn = partial(
         attr_membership,
         value_set=VALID_LAT_UNITS,
@@ -555,6 +564,15 @@ def get_longitude_variables(nc):
     for variable in nc.get_variables_by_attributes(standard_name="longitude"):
         longitude_variables.append(variable.name)
 
+    # Then axis
+    for variable in nc.get_variables_by_attributes(axis="X"):
+        if not (
+            variable.name in longitude_variables
+            or getattr(variable, "standard_name", None)
+            in {"projection_x_coordinate", "projection_x_angular_coordinate"}
+        ):
+            longitude_variables.append(variable.name)
+
     check_fn = partial(
         attr_membership,
         value_set=VALID_LON_UNITS,
@@ -912,7 +930,7 @@ def is_dataset_valid_ragged_array_repr_featureType(nc, feature_type: str):
             return False
     else:
         ftype = feature_type.lower()
-        if len(cf_role_vars) > 1:
+        if len(cf_role_vars) > 1 or not ftype:
             return False
 
     cf_role_var = nc.get_variables_by_attributes(cf_role=f"{ftype}_id")[0]
@@ -1027,6 +1045,7 @@ def resolve_ragged_array_dimension(ds: Dataset):
         ragged_type = "instance_dimension"
     if ragged_variable is None:
         raise ValueError("Could not find a ragged array related variable")
+    return ragged_type
 
 
 def is_variable_valid_ragged_array_repr_featureType(nc, variable: str) -> bool:


Test patch:
diff --git a/compliance_checker/tests/helpers.py b/compliance_checker/tests/helpers.py
index a07c1aa..436239b 100644
--- a/compliance_checker/tests/helpers.py
+++ b/compliance_checker/tests/helpers.py
@@ -13,7 +13,10 @@ class MockNetCDF(Dataset):
         # even though we aren't persisting data to disk, the constructor
         # requires a filename not currently in use by another Dataset object..
         if filename is None:
-            temp_filename = tempfile.NamedTemporaryFile(suffix=".nc", delete=True).name
+            temp_file = tempfile.NamedTemporaryFile(suffix=".nc", delete=True)
+            temp_filename = temp_file.name
+            # close temp file so that resources are released after tests
+            temp_file.close()
         else:
             temp_filename = filename
         super().__init__(
diff --git a/compliance_checker/tests/test_cf.py b/compliance_checker/tests/test_cf.py
index e5d2a8b..f1a3ddd 100644
--- a/compliance_checker/tests/test_cf.py
+++ b/compliance_checker/tests/test_cf.py
@@ -952,12 +952,9 @@ class TestCF1_6(BaseTestCase):
         assert len([r for r in results if r.value[0] < r.value[1]]) == 3
         assert (r.name == "§4.1 Latitude Coordinate" for r in results)
 
-        # check with another ds -- all 6 vars checked pass
         dataset = self.load_dataset(STATIC_FILES["rotated_pole_grid"])
         results = self.cf.check_latitude(dataset)
         scored, out_of, messages = get_results(results)
-        assert len(results) == 3
-        assert scored == out_of
         assert (r.name == "§4.1 Latitude Coordinate" for r in results)
 
         # hack to avoid writing to read-only file
@@ -997,7 +994,6 @@ class TestCF1_6(BaseTestCase):
         dataset = self.load_dataset(STATIC_FILES["rotated_pole_grid"])
         results = self.cf.check_latitude(dataset)
         scored, out_of, messages = get_results(results)
-        assert (scored, out_of) == (3, 3)
         # hack to avoid writing to read-only file
         dataset.variables["rlon"] = MockVariable(dataset.variables["rlon"])
         rlon = dataset.variables["rlon"]
@@ -2287,6 +2283,7 @@ class TestCF1_7(BaseTestCase):
         self.assertTrue(self.cf._process_v_datum_str("Ordnance Datum Newlyn", conn))
         # NAD83 isn't a vertical datum to begin with, expect failure
         self.assertFalse(self.cf._process_v_datum_str("NAD83", conn))
+        conn.close()
 
     def test_check_grid_mapping_crs_wkt(self):
         dataset = self.load_dataset(STATIC_FILES["mapping"])
@@ -3252,7 +3249,8 @@ 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)
-        assert not results[0].msgs
+        # "time" dimension named in domain variable not in dataset dimensions
+        assert results[0].msgs
         # failing example, coordinates for cell_measures variable are no longer subset
         domain_var.cell_measures = "volume: cube_bad"
         dataset.createVariable("cube_bad", "f8", ("lon", "lat", "depth", "time"))
diff --git a/compliance_checker/tests/test_cf_integration.py b/compliance_checker/tests/test_cf_integration.py
index a55da15..ec8b463 100644
--- a/compliance_checker/tests/test_cf_integration.py
+++ b/compliance_checker/tests/test_cf_integration.py
@@ -28,7 +28,7 @@ dataset_stem__expected_messages = [
     ),
     (
         "usgs_dem_saipan",
-        ['§2.6.1 Conventions global attribute does not contain "CF-1.8"'],
+        ['§2.6.1 Conventions global attribute does not contain "CF-1.9"'],
     ),
     (
         "l01-met",
@@ -86,7 +86,7 @@ dataset_stem__expected_messages = [
         "swan",
         [
             "global attribute _CoordSysBuilder should begin with a letter and be composed of letters, digits, and underscores",
-            '§2.6.1 Conventions global attribute does not contain "CF-1.8"',
+            '§2.6.1 Conventions global attribute does not contain "CF-1.9"',
             'Units "hours since 2013-02-18T00:00:00Z" for variable time_offset must be convertible to canonical units "s"',
             "lon's axis attribute must be T, X, Y, or Z, currently x",
             "lat's axis attribute must be T, X, Y, or Z, currently y",
diff --git a/compliance_checker/tests/test_feature_detection.py b/compliance_checker/tests/test_feature_detection.py
index 8f3d876..4afe9f0 100644
--- a/compliance_checker/tests/test_feature_detection.py
+++ b/compliance_checker/tests/test_feature_detection.py
@@ -300,7 +300,7 @@ class TestFeatureDetection(TestCase):
     def test_rotated_pole_grid(self):
         with Dataset(resources.STATIC_FILES["rotated_pole_grid"]) as nc:
             latitudes = util.get_latitude_variables(nc)
-            assert latitudes == ["lat"]
+            assert sorted(latitudes) == ["lat", "rlat"]
             assert util.is_mapped_grid(nc, "temperature") is True
 
     def test_vertical_coords(self):