Repository: ioos/compliance-checker
Pull request #967 (2022-10-26 22:52:29)

7.1 cell boundaries
Related to issue #952 



Patch:
diff --git a/compliance_checker/cf/appendix_d.py b/compliance_checker/cf/appendix_d.py
index c11c5d9..5e80ca5 100644
--- a/compliance_checker/cf/appendix_d.py
+++ b/compliance_checker/cf/appendix_d.py
@@ -73,7 +73,7 @@ dimless_vertical_coordinates_1_6 = {  # only for CF-1.6
         ocean_computed_standard_names,
     ),
     "ocean_sigma_z_coordinate": (
-        {"sigma", "eta", "depth", "depth_c", "zlev"},
+        {"sigma", "eta", "depth", "depth_c", "nsigma", "zlev"},
         ocean_computed_standard_names,
     ),
     "ocean_double_sigma_coordinate": (
diff --git a/compliance_checker/cf/cf_1_6.py b/compliance_checker/cf/cf_1_6.py
index 86d05ff..b31519a 100644
--- a/compliance_checker/cf/cf_1_6.py
+++ b/compliance_checker/cf/cf_1_6.py
@@ -2,7 +2,6 @@ import difflib
 import logging
 from collections import defaultdict
 
-import cftime
 import numpy as np
 import regex
 from cf_units import Unit
@@ -95,13 +94,6 @@ class CF1_6Check(CFNCCheck):
                         k, v.datatype
                     )
                 )
-            # TODO: DRY, don't repeat code path
-            if v.dtype is not str and v.dtype.kind == "S" and v.ndim not in (1, 2):
-                fails.append(
-                    "The fixed-length string variable {} must be one or "
-                    "two-dimensional, current length is {}".format(v.name, v.ndim)
-                )
-
         return Result(
             BaseCheck.HIGH,
             (total - len(fails), total),
@@ -412,73 +404,6 @@ class CF1_6Check(CFNCCheck):
                 )
         return valid_dimension_order.to_result()
 
-    def check_fill_value_equal_missing_value(self, ds):
-        """
-        If both missing_value and _FillValue be used, they should have the same value. 
-        This according to CF §2.5.1 Recommendations:
-        
-        :param netCDF4.Dataset ds: An open netCDF dataset
-        :rtype: list
-        :return: List of Results
-        """
-        fails = []
-        total = 0
-
-        for name, variable in ds.variables.items():
-            # If the variable have a defined _FillValue a defined missing_value check it.
-            
-            if hasattr(variable, "_FillValue") and hasattr(variable, "missing_value"):
-                total = total + 1
-                if variable._FillValue != variable.missing_value:
-                    fails.append(
-                        "For the variable {} the missing_value must be equal to the _FillValue".format(
-                        variable.name
-                        )
-                        )             
-                    
-        return Result(
-            BaseCheck.MEDIUM, 
-            (len(fails), total),
-            self.section_titles["2.5"], 
-            msgs=fails,
-        )
-    
-    def check_valid_range_or_valid_min_max_present(self,ds):
-        '''
-        The valid_range attribute must not be present if the valid_min
-        and/or valid_max attributes are present. This according to 2.5.1 Requirements.
-
-        :param netCDF4.Dataset ds: An open netCDF dataset
-        :rtype: list
-        :return: List of Results
-        '''
-        fails = []
-        total = 0
-
-        for name, variable in ds.variables.items():
-            
-            if (hasattr(variable, "valid_max") 
-                and (hasattr(variable, "valid_min") 
-                or hasattr(variable, "valid_range")
-                )
-                ):
-
-                total = total + 1
-            
-                fails.append(
-                    "For the variable {} the valid_range attribute must not be present "
-                    "if the valid_min and/or valid_max attributes are present".format(
-                    variable.name
-                    )
-                    )               
-                    
-        return Result(
-            BaseCheck.MEDIUM, 
-            (len(fails), total),
-            self.section_titles["2.5"], 
-            msgs=fails,
-        )
-    
     def check_fill_value_outside_valid_range(self, ds):
         """
         Checks each variable's _FillValue to ensure that it's in valid_range or
@@ -1893,69 +1818,25 @@ class CF1_6Check(CFNCCheck):
 
         ret_val = []
 
-        def check_standard_calendar_no_cross(time_var):
-            """
-            Check that the time variable does not cross the date
-            1582-10-15 when standard or gregorian calendars are used
-            """
-            # IMPLEMENTATION CONFORMANCE 4.4.1 RECOMMENDED 2/2
-            # Only get non-nan/FillValue times, as these are the only things
-            # that make sense for conversion.  Furthermore, non-null checks
-            # should be made for time coordinate variables anyways, so errors
-            # should be caught where implemented there
-            crossover_date = cftime.DatetimeGregorian(1582, 10, 15)
-            times = cftime.num2date(time_var[:], time_var.units)
-
-            # appears to be necessary to override supplied _FillValue of "?"
-            # FillValues shouldn't fail, so set to crossover date to pass
-            times.set_fill_value(crossover_date)
-            no_cross_1582 = ~np.any(times < crossover_date)
-            if no_cross_1582:
-                reasoning = (
-                    f"Variable {time_var.name} has standard/gregorian "
-                    "calendar and does not cross 1582-10-15T00:00Z"
-                )
-            else:
-                reasoning = (
-                    f"Variable {time_var.name} has time values "
-                    "prior to 1582-10-15T00:00Z and utilizes "
-                    "the standard/gregorian calendar"
-                )
-
-            return Result(
-                BaseCheck.LOW, no_cross_1582, self.section_titles["4.4"], [reasoning]
-            )
-
         # if has a calendar, check that it is within the valid values
         # otherwise no calendar is valid
-
         for time_var in ds.get_variables_by_attributes(
             calendar=lambda c: c is not None
         ):
-            if time_var.calendar == "gregorian":
-                reasoning = (
-                    f"For time variable {time_var.name}, when using "
-                    "the standard Gregorian calendar, the value "
-                    '"standard" is preferred over "gregorian" for '
-                    "the calendar attribute"
-                )
-                result = Result(
-                    BaseCheck.LOW, False, self.section_titles["4.4"], [reasoning]
-                )
-                ret_val.append(result)
-                # check here and in the below case that time does not cross
-                # thee date 1582-10-15 as requested by CF conformance
-                ret_val.append(check_standard_calendar_no_cross(time_var))
-            elif time_var.calendar == "standard":
-                ret_val.append(check_standard_calendar_no_cross(time_var))
+            reasoning = None
+            standard_calendar = time_var.calendar in valid_calendars
+
             # if a nonstandard calendar, then leap_years and leap_months must
             # must be present
-            if time_var.calendar not in valid_calendars:
+            if not standard_calendar:
                 result = self._check_leap_time(time_var)
             # passes if the calendar is valid, otherwise notify of invalid
             # calendar
             else:
-                result = Result(BaseCheck.LOW, True, self.section_titles["4.4"], None)
+                result = Result(
+                    BaseCheck.LOW, True, self.section_titles["4.4"], reasoning
+                )
+
             ret_val.append(result)
 
         return ret_val
diff --git a/compliance_checker/cf/cf_1_7.py b/compliance_checker/cf/cf_1_7.py
index 0729dac..d77796f 100644
--- a/compliance_checker/cf/cf_1_7.py
+++ b/compliance_checker/cf/cf_1_7.py
@@ -243,6 +243,10 @@ class CF1_7Check(CF1_6Check):
             variable = ds.variables[variable_name]
             valid = True
             reasoning = []
+
+            # 7.1 Required 1/5:
+            # The type of the bounds attribute is a string whose value is a single variable name.
+            # The specified variable must exist in the file.
             if boundary_variable_name not in ds.variables:
                 valid = False
                 reasoning.append(
@@ -253,6 +257,8 @@ class CF1_7Check(CF1_6Check):
                 )
             else:
                 boundary_variable = ds.variables[boundary_variable_name]
+
+            # 7.1 Required 2/5:
             # The number of dimensions in the bounds variable should always be
             # the number of dimensions in the referring variable + 1
             if boundary_variable.ndim < 2:
@@ -285,7 +291,8 @@ class CF1_7Check(CF1_6Check):
                     "".format(variable.name, boundary_variable.dimensions)
                 )
 
-            # ensure p vertices form a valid simplex given previous a...n
+            # 7.1 Required 2/5: continue
+            # Ensure p vertices form a valid simplex given previous a...n
             # previous auxiliary coordinates
             if (
                 ds.dimensions[boundary_variable.dimensions[-1]].size
@@ -301,6 +308,38 @@ class CF1_7Check(CF1_6Check):
                     )
                 )
 
+            # 7.1 Required 3/5:
+            # A boundary variable must be a numeric data type
+            if boundary_variable.dtype.kind not in "biufc":
+                valid = False
+                reasoning.append(
+                    "Boundary variable {} specified by {}".format(
+                        boundary_variable.name, variable.name
+                    )
+                    + "must be a numeric data type "
+                )
+
+            # 7.1 Required 4/5:
+            # If a boundary variable has units, standard_name, axis, positive, calendar, leap_month,
+            # leap_year or month_lengths attributes, they must agree with those of its associated variable.
+            if boundary_variable.__dict__.keys():
+                for item in boundary_variable.__dict__.keys():
+                    if hasattr(variable, item):
+                        if getattr(variable, item) != getattr(boundary_variable, item):
+                            valid = False
+                            reasoning.append(
+                                "'{}' has attr '{}' with value '{}' that does not agree "
+                                "with its associated variable ('{}')'s attr value '{}'"
+                                "".format(
+                                    boundary_variable_name,
+                                    item,
+                                    getattr(boundary_variable, item),
+                                    variable.name,
+                                    getattr(variable, item),
+                                )
+                            )
+
+            # 7.1 Required 5/5:
             # check if formula_terms is present in the var; if so,
             # the bounds variable must also have a formula_terms attr
             if hasattr(variable, "formula_terms"):
@@ -312,12 +351,82 @@ class CF1_7Check(CF1_6Check):
                         )
                     )
 
+            # 7.1 Recommendations 2/2
+            # Boundary variables should not have the _FillValue, missing_value, units, standard_name, axis,
+            # positive, calendar, leap_month, leap_year or month_lengths attributes.
+            attributes_to_check = {
+                "_FillValue",
+                "missing_value",
+                "units",
+                "standard_name",
+                "axis",
+                "positive",
+                "calendar",
+                "leap_month",
+                "leap_year",
+                "month_lengths",
+            }
+            if boundary_variable.__dict__.keys():
+                lst1 = boundary_variable.__dict__.keys()
+                lst2 = attributes_to_check
+                unwanted_attributes = [value for value in lst1 if value in lst2]
+                if unwanted_attributes:
+                    valid = False
+                    reasoning.append(
+                        "The Boundary variables '{}' should not have the attributes: '{}'".format(
+                            boundary_variable_name, unwanted_attributes
+                        )
+                    )
+
             result = Result(
                 BaseCheck.MEDIUM, valid, self.section_titles["7.1"], reasoning
             )
             ret_val.append(result)
         return ret_val
 
+    def check_cell_boundaries_interval(self, ds):
+        """
+        7.1 Cell Boundaries
+        Recommendations: (1/2)
+        The points specified by a coordinate or auxiliary coordinate variable
+        should lie within, or on the boundary, of the cells specified by the
+        associated boundary variable.
+        """
+        ret_val = []
+        reasoning = []
+        for variable_name, boundary_variable_name in cfutil.get_cell_boundary_map(
+            ds
+        ).items():
+            valid = True
+
+            variable = ds.variables[variable_name]
+            boundary_variable = ds.variables[boundary_variable_name]
+
+            for ii in range(len(variable[:])):
+                if abs(boundary_variable[ii][1]) >= abs(boundary_variable[ii][0]):
+                    if not (
+                        (abs(variable[ii]) >= abs(boundary_variable[ii][0]))
+                        and (abs(variable[ii]) <= abs(boundary_variable[ii][1]))
+                    ):
+                        valid = False
+                        reasoning.append(
+                            "The points specified by the coordinate variable {} ({})"
+                            " lie outside the boundary of the cell specified by the "
+                            "associated boundary variable {} ({})".format(
+                                variable_name,
+                                variable[ii],
+                                boundary_variable_name,
+                                boundary_variable[ii],
+                            )
+                        )
+
+                result = Result(
+                    BaseCheck.MEDIUM, valid, self.section_titles["7.1"], reasoning
+                )
+                ret_val.append(result)
+                print(ret_val)
+            return ret_val
+
     def check_cell_measures(self, ds):
         """
         A method to over-ride the CF1_6Check method. In CF 1.7, it is specified


Test patch:
diff --git a/compliance_checker/tests/test_cf.py b/compliance_checker/tests/test_cf.py
index 1ce6248..ee2507f 100644
--- a/compliance_checker/tests/test_cf.py
+++ b/compliance_checker/tests/test_cf.py
@@ -150,35 +150,6 @@ class TestCF1_6(BaseTestCase):
         dataset = self.load_dataset(STATIC_FILES["bad_data_type"])
         result = self.cf.check_data_types(dataset)
 
-        dataset = MockTimeSeries()
-        # test dimensionality of string
-        dataset.createDimension("num_strings", 3)
-        dataset.createDimension("max_string_length", 80)
-        dataset.createDimension("extra", 9)
-
-        # TEST CONFORMANCE 2.2 1/2
-        # create strings with no dimensions -- fail
-        dataset.createVariable("zero_dimensional", "S1", ())
-        # create strings with one dimension -- valid
-        dataset.createVariable("one_dimensional", "S1", ("max_string_length"))
-        # create strings with two dimensions - valid
-        # NB: string length is supposed to be last, but this can't be inferred
-        #     easily with
-        dataset.createVariable(
-            "two_dimensional", "S1", ("num_strings", "max_string_length")
-        )
-        # create strings with three dimensions - valid
-        dataset.createVariable(
-            "three_dimensional", "S1", ("num_strings", "max_string_length", "extra")
-        )
-
-        result = self.cf.check_data_types(dataset)
-        expected_msgs = [
-            f"The fixed-length string variable {dim_name}_dimensional must "
-            f"be one or two-dimensional, current length is {dim_len}"
-            for dim_name, dim_len in (("zero", 0), ("three", 3))
-        ]
-        assert result.msgs == expected_msgs
         # TODO
         # the acdd_reformat_rebase branch has a new .nc file
         # which constructs the temp variable with an int64 dtype --
@@ -382,77 +353,7 @@ class TestCF1_6(BaseTestCase):
         result = self.cf.check_dimension_order(dataset)
         self.assertEqual((3, 3), result.value)
         self.assertEqual([], result.msgs)
-        
-    def test_check_fill_value_equal_missing_value(self):
-        """
-        According to CF §2.5.1 Recommendations: If both missing_value and _FillValue be used, 
-        they should have the same value.  
-        """
-        # TEST CONFORMANCE 2.5.1 RECOMMENDED
-        dataset = MockTimeSeries()
-        # Case of _FillValue and missing_value are not equal
-        dataset.createVariable("a", "d", ("time",), fill_value=9999.9)
-        dataset.variables["a"][0] = 1
-        dataset.variables["a"][1] = 2   
-        dataset.variables["a"].setncattr("missing_value", [9939.9])
 
-        # Case of _FillValue and missing_value are equal
-        dataset.createVariable("b", "d", ("time",), fill_value=9999.9)
-        dataset.variables["b"][0] = 1
-        dataset.variables["b"][1] = 2   
-        dataset.variables["b"].setncattr("missing_value", [9999.9])
-
-
-        result = self.cf.check_fill_value_equal_missing_value(dataset)
-        
-        # check if the test fails when when variable "a" is checked.
-        expected_msgs = [
-            f"For the variable {v_name} the missing_value must be equal to the _FillValue"
-            for v_name in ("a")]
- 
-        assert result.msgs == expected_msgs 
-
-    def test_check_valid_range_or_valid_min_max_present(self):
-        """
-        2.5.1 Missing data, valid and actual range of data
-        Requirements:
-        The valid_range attribute must not be present if the 
-        valid_min and/or valid_max attributes are present.
-        """
-        # TEST CONFORMANCE 2.5.1 REQUIRED
-        dataset = MockTimeSeries()
-        # Case of valid_min, valid_max, and valid_range are present
-        dataset.createVariable("a", "d", ("time",), fill_value=9999.9)
-        dataset.variables["a"][0] = 1
-        dataset.variables["a"][1] = 2   
-        dataset.variables["a"].setncattr("valid_min", [-10])
-        dataset.variables["a"].setncattr("valid_max", [10])
-        dataset.variables["a"].setncattr("valid_range", [-10, 10])
-
-        # Case of valid_min and valid_max are present and valid_range is absent
-        dataset.createVariable("b", "d", ("time",), fill_value=9999.9)
-        dataset.variables["b"][0] = 1
-        dataset.variables["b"][1] = 2   
-        dataset.variables["a"].setncattr("valid_min", [-10])
-        dataset.variables["a"].setncattr("valid_max", [10])
-
-        # Case of valid_min and valid_max are absent and valid_range is present
-        dataset.createVariable("c", "d", ("time",), fill_value=9999.9)
-        dataset.variables["c"][0] = 1
-        dataset.variables["c"][1] = 2   
-        dataset.variables["c"].setncattr("valid_range", [-10, 10]) 
-
-        result = self.cf.check_valid_range_or_valid_min_max_present(dataset)
-        
-        # check if the test fails when when variable "a" is checked.
-        expected_msgs = [
-            f"For the variable {v_name} the valid_range attribute must not be present "
-            f"if the valid_min and/or valid_max attributes are present"
-            for v_name in ("a")]
- 
-        assert result.msgs == expected_msgs
-        assert result.value[0] == result.value[1]
-    
     def test_check_fill_value_outside_valid_range(self):
         """
         2.5.1 The _FillValue should be outside the range specified by valid_range (if used) for a variable.
@@ -1159,7 +1060,7 @@ class TestCF1_6(BaseTestCase):
         self.assertTrue(
             no_missing_terms(
                 "ocean_sigma_z_coordinate",
-                {"sigma", "eta", "depth", "depth_c", "zlev"},
+                {"sigma", "eta", "depth", "depth_c", "nsigma", "zlev"},
                 dimless_vertical_coordinates_1_6,
             )
         )
@@ -1293,7 +1194,7 @@ class TestCF1_6(BaseTestCase):
         assert bad_month_msg in messages
 
         dataset.variables["time"].month_lengths = np.array(
-            [31, 29, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31], dtype=int
+            [31, 29, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31], dtype=np.uint8
         )
         results = self.cf.check_calendar(dataset)
         scored, out_of, messages = get_results(results)
@@ -1336,34 +1237,6 @@ class TestCF1_6(BaseTestCase):
         scored, out_of, messages = get_results(results)
         assert leap_year_msg not in messages
 
-        for calendar in ("standard", "gregorian"):
-            dataset.variables["time"].calendar = calendar
-            dataset.variables["time"].units = "seconds since 1582-10-15T00:00Z"
-            # 500 element array with some failing values
-            # TEST CONFORMANCE 4.4.1 RECOMMENDED 4/4
-            dataset.variables["time"][:] = np.arange(-2, 498)
-            results = self.cf.check_calendar(dataset)
-            scored, out_of, messages = get_results(results)
-            assert messages[-1] == (
-                "Variable time has time values prior to "
-                "1582-10-15T00:00Z and utilizes the "
-                "standard/gregorian calendar"
-            )
-            dataset.variables["time"][:] = np.arange(0, 500)
-            results = self.cf.check_calendar(dataset)
-            scored, out_of, messages = get_results(results)
-            assert messages[-1] == (
-                "Variable time has standard/gregorian "
-                "calendar and does not cross 1582-10-15T00:00Z"
-            )
-            # TEST CONFORMANCE 4.4.1 RECOMMENDED 3/4
-            if calendar == "gregorian":
-                assert (
-                    "For time variable time, when using the standard "
-                    'Gregorian calendar, the value "standard" is preferred '
-                    'over "gregorian" for the calendar attribute' in messages
-                )
-
     def test_check_aux_coordinates(self):
         dataset = self.load_dataset(STATIC_FILES["illegal-aux-coords"])
         results = self.cf.check_aux_coordinates(dataset)
@@ -1985,7 +1858,7 @@ class TestCF1_7(BaseTestCase):
         dataset = self.load_dataset(STATIC_FILES["grid-boundaries"])
         results = self.cf.check_cell_boundaries(dataset)
         score, out_of, messages = get_results(results)
-        assert (score, out_of) == (2, 2)
+        assert (score, out_of) == (0, 2)
 
         dataset = self.load_dataset(STATIC_FILES["cf_example_cell_measures"])
         results = self.cf.check_cell_boundaries(dataset)
@@ -2023,6 +1896,38 @@ class TestCF1_7(BaseTestCase):
                 in messages
             )
 
+    def test_check_cell_boundaries_interval(self):
+        """
+        7.1 Cell Boundaries
+        Recommendations: (1/2)
+        The points specified by a coordinate or auxiliary coordinate variable
+        should lie within, or on the boundary, of the cells specified by the
+        associated boundary variable.
+        """
+
+        # create Cells on a longitude axis
+        dataset = MockTimeSeries()
+        dataset.createDimension("rnv", 2)
+        dataset.createDimension("rlon", 2)
+        dataset.createVariable("rlon", "d", ("rlon",))
+        dataset.createVariable("rlon_bnds", "d", ("rlon", "rnv"))
+
+        rlon = dataset.variables["rlon"]
+        rlon.standard_name = "longitude"
+        rlon.units = "degrees_east"
+        rlon.axis = "X"
+        rlon.long_name = "Longitude"
+        rlon.bounds = "rlon_bnds"
+        rlon[:] = np.array([-97.5, -99.5], dtype=np.float64)
+
+        rlon_bnds = dataset.variables["rlon_bnds"]
+        rlon_bnds.long_name = "Longitude Cell Boundaries"
+        rlon_bnds[:] = np.array([[-97, -98], [-98, -99]], dtype=np.float64)
+
+        results = self.cf.check_cell_boundaries_interval(dataset)
+        score, out_of, messages = get_results(results)
+        assert (score, out_of) == (1, 2)
+
     def test_cell_measures(self):
         """Over-ride the test_cell_measures from CF1_6"""