Repository: ioos/compliance-checker
Pull request #708 (2019-11-27 19:39:11)

Parent attr compare fixup


Patch:
diff --git a/compliance_checker/cf/cf.py b/compliance_checker/cf/cf.py
index e3c0234..4a44b82 100644
--- a/compliance_checker/cf/cf.py
+++ b/compliance_checker/cf/cf.py
@@ -387,22 +387,8 @@ class CFBaseCheck(BaseCheck):
 
         return valid_formula_terms.to_result()
 
-    # TODO, move this into alphabetical order?
-    def check_grid_mapping_attr_type(self, attr_name, attr, attr_type, variable=None):
-        """
-        Check the type of an attribute. Wrapper function, designed to
-        be overloaded in concrete implementations.
 
-        :param str attr_name: name of attribute
-        :param attr: attribute to check
-        :param str attr_type: designated type of attr; 'S', 'N', or 'D'
-        :param netCDF4.Variable variable
-        :rtype tuple
-        :return two-tuple of (bool, str|None)
-        """
-        raise NotImplementedError
-
-    def check_grid_mapping_attr_condition(self, attr, attr_name, ret_val):
+    def _check_grid_mapping_attr_condition(self, attr, attr_name, ret_val):
         """
         Evaluate a condition (or series of conditions) for a particular
         attribute. Designed to be overloaded in subclass implementations.
@@ -426,6 +412,32 @@ class CFBaseCheck(BaseCheck):
         dimension_string = ''.join(dimension_order)
         return regx.match(dimension_string) is not None
 
+    def _parent_var_attr_type_check(self, attr_name, var, ctx):
+        """
+        Checks that an attribute has an equivalent value to a parent variable.
+        Takes an attribute name, variable, and test context on which to operate.
+        :param str attr_name: The name of the attribute to be checked
+        :param netCDF4.Variable var: The variable against which to be checked
+        :param compliance_checker.base.TestCtx ctx: The associated test context to modify
+        :rtype None
+        :return None
+        """
+        attr_val = var.getncattr(attr_name)
+
+        if isinstance(attr_val, basestring):
+            type_match = var.dtype.kind == 'S'
+            val_type = type(attr_val)
+        else:
+            val_type = attr_val.dtype.type
+            type_match = val_type == var.dtype.type
+
+        ctx.assert_true(type_match,
+            "Attribute '{}' (type: {}) and parent variable '{}' (type: {}) "
+            "must have equivalent datatypes".format(attr_name,
+                                                    val_type,
+                                                    var.name,
+                                                    var.dtype.type))
+
     def _find_aux_coord_vars(self, ds, refresh=False):
         '''
         Returns a list of auxiliary coordinate variables
@@ -937,7 +949,7 @@ class CFBaseCheck(BaseCheck):
 
         return results
 
-    def _check_attr_type(self, attribute, attr_type, variable=None):
+    def _check_attr_type(self, attr_name, attr_type, attribute, variable=None):
         """
         Check if an attribute `attr` is of the type `attr_type`. Upon getting
         a data type of 'D', the attr must have the same data type as the
@@ -947,20 +959,19 @@ class CFBaseCheck(BaseCheck):
         numeric types, and 'D' requires the attribute type match the type
         of the variable it is assigned to.
 
-        :param attribute: attribute to check
+        :param str attr_name: name of attr being checked (to format message)
         :param str attr_type: the correct type of the attribute
+        :param attribute: attribute to check
         :param variable: if given, type should match attr
-        :rtype list
-        :return A two-element list that contains pass/fail status as a boolean and
+        :rtype tuple
+        :return A two-tuple that contains pass/fail status as a boolean and
                 a message string (or None if unset) as the second element.
-                The string will contain a format field to be filled with a
-                string describing the attribute.
         """
 
         if attr_type == 'S':
             if not isinstance(attribute, basestring):
                 return [False,
-                        "{} must be a string"]
+                        "{} must be a string".format(attr_name)]
         else:
             # if it's not a string, it should have a numpy dtype
             underlying_dtype = getattr(attribute, 'dtype', None)
@@ -968,25 +979,28 @@ class CFBaseCheck(BaseCheck):
             # TODO check for np.nan separately
             if underlying_dtype is None:
                 return [False,
-                        "{} must be a numeric type"]
+                        "{} must be a numeric type".format(attr_name)]
 
             # both D and N should be some kind of numeric value
             is_numeric = np.issubdtype(underlying_dtype, np.number)
             if attr_type == 'N':
                 if not is_numeric:
                     return [False,
-                            "{} must be a numeric type"]
+                            "{} must be a numeric type".format(attr_name)]
             elif attr_type == 'D':
                 # TODO: handle edge case where variable is unset here
+                temp_ctx = TestCtx()
+                self._parent_var_attr_type_check(attr_name, variable, temp_ctx)
                 var_dtype = getattr(variable, 'dtype', None)
-                if (underlying_dtype != var_dtype):
-                    return [False,
-                            "{{}} must be numeric and must match {} dtype".format(var_dtype)]
+                if temp_ctx.messages:
+                    return (False,
+                            "{} must be numeric and must be equivalent to {} dtype".format(attr_name, var_dtype))
             else:
                 # If we reached here, we fell off with an unrecognized type
-                return [False, "{{}} has unrecognized type '{}'".format(attr_type)]
+                return (False, "{} has unrecognized type '{}'".format(attr_name,
+                                                                      attr_type))
         # pass if all other possible failure conditions have been evaluated
-        return [True, None]
+        return (True, None)
 
     def _handle_dtype_check(self, attribute, attr_name, attr_dict,
                             variable=None):
@@ -1012,7 +1026,10 @@ class CFBaseCheck(BaseCheck):
                                                               variable.name))
 
         # check the type
-        return_value = self._check_attr_type(attribute, attr_type, variable)
+        return_value = self._check_attr_type(attr_name,
+                                             attr_type,
+                                             attribute,
+                                             variable)
 
         # if the second element is a string, format it
         if isinstance(return_value[1], basestring):
@@ -1157,22 +1174,8 @@ class CF1_6Check(CFNCCheck):
         }
 
         for var_name, var in ds.variables.items():
-            for att in special_attrs.intersection(var.ncattrs()):
-                val = var.getncattr(att)
-                if isinstance(val, basestring):
-                    type_match = var.dtype.kind == 'S'
-                    val_type = type(val)
-                else:
-                    val_type = val.dtype.type
-                    type_match = val_type == var.dtype.type
-
-                ctx.assert_true(type_match,
-                    "Attribute '{}' (type: {}) and parent variable '{}' (type: {}) "
-                    "must have equivalent datatypes".format(att,
-                                                            val_type,
-                                                            var_name,
-                                                            var.dtype.type)
-                    )
+            for att_name in special_attrs.intersection(var.ncattrs()):
+                self._parent_var_attr_type_check(att_name, var, ctx)
         return ctx.to_result()
 
     def check_naming_conventions(self, ds):
@@ -2803,30 +2806,8 @@ class CF1_6Check(CFNCCheck):
 
         return ret_val
 
-    def check_grid_mapping_attr_type(self, attr_name, attr, attr_type, variable=None):
-        """
-        Check the type of an attribute. Wrapper function,
-        implemented for CF-1.6.
 
-        :param str attr_name: name of attribute
-        :param attr: attribute to check
-        :param str attr_type: designated type of attr; 'S', 'N', or 'D'
-        :param netCDF4.Variable variable
-        :rtype tuple
-        :return two-tuple of (bool, str|None)
-        """
-
-        # get check result, list [bool, str|None]
-        _res = self._check_attr_type(attr, attr_type, variable)
-
-        # fully format the message if it needs it
-        if isinstance(_res[1], basestring):
-            _res[1] = _res[1].format(attr_name)
-
-        # tuple for immutability
-        return tuple(_res)
-
-    def check_grid_mapping_attr_condition(self, attr, attr_name):
+    def _check_grid_mapping_attr_condition(self, attr, attr_name):
         """
         Evaluate a condition (or series of conditions) for a particular
         attribute. Implementation for CF-1.6.
@@ -2986,8 +2967,6 @@ class CF1_6Check(CFNCCheck):
             'straight_vertical_longitude_from_pole must satisfy (-180 <= x <= 180)'
         )
 
-    # grid mapping dictionary, appendix F
-
 
     ###############################################################################
     # Chapter 6: Labels and Alternative Coordinates
@@ -4170,7 +4149,7 @@ class CF1_7Check(CF1_6Check):
 
         return ret_val
 
-    def check_grid_mapping_attr_condition(self, attr, attr_name):
+    def _check_grid_mapping_attr_condition(self, attr, attr_name):
         """
         Evaluate a condition (or series of conditions) for a particular
         attribute. Implementation for CF-1.7.
@@ -4206,7 +4185,7 @@ class CF1_7Check(CF1_6Check):
             return self._evaluate_towgs84(attr)
 
         else: # invoke method from 1.6, as these names are all still valid
-            return super(CF1_7Check, self).check_grid_mapping_attr_condition(attr, attr_name)
+            return super(CF1_7Check, self)._check_grid_mapping_attr_condition(attr, attr_name)
 
     def _check_gmattr_existence_condition_geoid_name_geoptl_datum_name(self, var):
         """


Test patch:
diff --git a/compliance_checker/tests/test_cf.py b/compliance_checker/tests/test_cf.py
index f318a61..26073b4 100644
--- a/compliance_checker/tests/test_cf.py
+++ b/compliance_checker/tests/test_cf.py
@@ -163,7 +163,7 @@ class TestCF1_6(BaseTestCase):
         flat_messages = {msg for res in aa_results for msg in res.msgs}
         self.assertIn('Attribute compress should not be in variable non-coordinate attributes for variable temp. Valid location(s) are [C]',
                       flat_messages)
-        self.assertIn('Attribute add_offset in variable temp must be a numeric type',
+        self.assertIn('add_offset must be a numeric type',
                       flat_messages)
 
     def test_naming_conventions(self):
@@ -905,9 +905,11 @@ class TestCF1_6(BaseTestCase):
         expected_name = u'§5.6 Horizontal Coorindate Reference Systems, Grid Mappings, Projections'
         assert all(r.name == expected_name for r in results.values())
 
-    def test_check_grid_mapping_attr_type(self):
+    # TODO: overhaul to use netCDF global attributes or mocks and variable
+    #       attributes
+    def test_check_attr_type(self):
         """
-        Check that the check_grid_mapping_attr_type method checks
+        Check that the check_attr_type method checks
         grid_mapping attribute types correctly.
         """
 
@@ -915,37 +917,34 @@ class TestCF1_6(BaseTestCase):
         att_name = 'test_att'
         att = np.int64(45)
         att_type = 'N' # numeric
-        res = self.cf.check_grid_mapping_attr_type(att_name, att, att_type)
+        res = self.cf._check_attr_type(att_name, att_type, att)
         self.assertTrue(res[0])
         self.assertEqual(res[1], None)
 
+        # create a temporary variable and test this only
+        nc_obj = MockTimeSeries()
+        nc_obj.createVariable('temperature', 'd', ('time',))
+        nc_obj.variables['temperature'].setncattr('test_att', np.float64(45))
         att_name = 'test_att'
-        att = np.int64(45)
-        x = np.int64(45) # the variable
+        _var = nc_obj.variables['temperature']
+        att = np.float64(45)
         att_type = 'D' # numeric, types should match
-        res = self.cf.check_grid_mapping_attr_type(att_name, att, att_type, x)
+        res = self.cf._check_attr_type(att_name, att_type, att, _var)
         self.assertTrue(res[0])
         self.assertEqual(res[1], None)
 
         att_name = 'test_att'
         att = 'yo'
         att_type = 'S' # string
-        res = self.cf.check_grid_mapping_attr_type(att_name, att, att_type)
+        res = self.cf._check_attr_type(att_name, att_type, att)
         self.assertTrue(res[0])
         self.assertEqual(res[1], None)
 
-        att_name = 'test_att'
-        att = np.float64(45)
-        att_type = 'D' # string, types should match
-        res = self.cf.check_grid_mapping_attr_type(att_name, att, att_type, np.float64(95))
-        self.assertTrue(res[0])
-        self.assertEqual(res[1], None)
-        
         # test bad
         att_name = 'test_att'
         att = np.int64(45)
         att_type = 'S' # string, but att type is numeric
-        res = self.cf.check_grid_mapping_attr_type(att_name, att, att_type)
+        res = self.cf._check_attr_type(att_name, att_type, att)
         self.assertFalse(res[0])
         self.assertEqual(res[1], "test_att must be a string")
 
@@ -953,16 +952,21 @@ class TestCF1_6(BaseTestCase):
         att_name = 'test_att'
         att = 'bad'
         att_type = 'N' # numeric, but att type is string
-        res = self.cf.check_grid_mapping_attr_type(att_name, att, att_type)
+        res = self.cf._check_attr_type(att_name, att_type, att)
         self.assertFalse(res[0])
         self.assertEqual(res[1], "test_att must be a numeric type")
 
+        # create a temporary variable and test this only
+        nc_obj = MockTimeSeries()
+        nc_obj.createVariable('temperature', 'd', ('time',))
+        nc_obj.variables['temperature'].setncattr('test_att', np.int32(45))
+        _var = nc_obj.variables['temperature']
         att_name = 'test_att'
         att = np.int32(2)
         att_type = 'D' # should be same datatypes
-        res = self.cf.check_grid_mapping_attr_type(att_name, att, att_type, np.float32(2))
+        res = self.cf._check_attr_type(att_name, att_type, att, _var)
         self.assertFalse(res[0])
-        self.assertEqual(res[1], "test_att must be numeric and must match float32 dtype")
+        self.assertEqual(res[1], "test_att must be numeric and must be equivalent to float64 dtype")
 
     def test_check_grid_mapping_attr_condition(self):
         """
@@ -972,37 +976,37 @@ class TestCF1_6(BaseTestCase):
         # test passes
         attr_name = 'latitude_of_projection_origin'
         val = 0
-        res = self.cf.check_grid_mapping_attr_condition(val, attr_name)
+        res = self.cf._check_grid_mapping_attr_condition(val, attr_name)
         self.assertTrue(res[0])
 
         attr_name = 'longitude_of_projection_origin'
         val = 0
-        res = self.cf.check_grid_mapping_attr_condition(val, attr_name)
+        res = self.cf._check_grid_mapping_attr_condition(val, attr_name)
         self.assertTrue(res[0])
 
         attr_name = 'longitude_of_prime_meridian'
         val = 0
-        res = self.cf.check_grid_mapping_attr_condition(val, attr_name)
+        res = self.cf._check_grid_mapping_attr_condition(val, attr_name)
         self.assertTrue(res[0])
 
         attr_name = 'scale_factor_at_central_meridian'
         val = 1
-        res = self.cf.check_grid_mapping_attr_condition(val, attr_name)
+        res = self.cf._check_grid_mapping_attr_condition(val, attr_name)
         self.assertTrue(res[0])
 
         attr_name = 'scale_factor_at_projection_origin'
         val = 1
-        res = self.cf.check_grid_mapping_attr_condition(val, attr_name)
+        res = self.cf._check_grid_mapping_attr_condition(val, attr_name)
         self.assertTrue(res[0])
 
         attr_name = 'standard_parallel'
         val = 0
-        res = self.cf.check_grid_mapping_attr_condition(val, attr_name)
+        res = self.cf._check_grid_mapping_attr_condition(val, attr_name)
         self.assertTrue(res[0])
 
         attr_name = 'straight_vertical_longitude_from_pole'
         val = 0
-        res = self.cf.check_grid_mapping_attr_condition(val, attr_name)
+        res = self.cf._check_grid_mapping_attr_condition(val, attr_name)
         self.assertTrue(res[0])
 
     def test_check_geographic_region(self):
@@ -1667,40 +1671,48 @@ class TestCF1_7(BaseTestCase):
          Ensure the _check_attr_type method works as expected.
         '''
 
+        # create a temporary variable and test this only
+        nc_obj = MockTimeSeries()
+        nc_obj.createVariable('temperature', 'd', ('time',))
+        nc_obj.variables['temperature'].setncattr('test_att', np.float64(45))
+        att_name = 'test_att'
+        _var = nc_obj.variables['temperature']
+
         # first, test all valid checks show that it's valid
         attr = 'my_attr_value' # string
         attr_type =  'S'
-        result = self.cf._check_attr_type(attr, attr_type)
+        result = self.cf._check_attr_type(att_name, attr_type, attr)
         self.assertTrue(result[0])
 
         attr = np.int64(1)
         attr_type =  'N'
-        self.assertTrue(self.cf._check_attr_type(attr, attr_type)[0])
+        self.assertTrue(self.cf._check_attr_type(att_name, attr_type, attr)[0])
 
-        attr = np.int64(45) # numpy int
+        attr = np.float64(45)
         attr_type = 'D'
-        variable = np.int64(45)
         self.assertTrue(self.cf._check_attr_type(
-            attr, attr_type, variable)[0])
+            att_name, attr_type, attr, _var)[0])
 
         # check failures
         attr = 'my_attr_value'
         attr_type = 'N' # should be numeric
-        self.assertFalse(self.cf._check_attr_type(attr, attr_type)[0])
+        self.assertFalse(self.cf._check_attr_type(att_name, attr_type, attr)[0])
 
         attr = np.int(64)
         attr_type = 'S' # should be string
-        self.assertFalse(self.cf._check_attr_type(attr, attr_type)[0])
+        self.assertFalse(self.cf._check_attr_type(att_name, attr_type, attr)[0])
 
+        nc_obj = MockTimeSeries()
+        nc_obj.createVariable('temperature', 'd', ('time',))
+        nc_obj.variables['temperature'].setncattr('test_att', np.int32(45))
+        _var = nc_obj.variables['temperature']
         attr = np.int32(45)
         attr_type = 'D' # should match
-        variable = np.int64(45) # mismatching numpy types
-        self.assertFalse(self.cf._check_attr_type(
-            attr, attr_type, variable)[0])
+        self.assertFalse(self.cf._check_attr_type(att_name, attr_type, attr, _var)[0])
 
     def test_check_grid_mapping_attr_condition(self):
         """
-        Ensure the CF-1.7 implementation of check_grid_mapping_attr_condition()
+        Ensure the CF-1.7 implementation of _check_grid_mapping_attr_condition()
         works as expected.
         """
 
@@ -1708,104 +1720,104 @@ class TestCF1_7(BaseTestCase):
 
         att_name = 'horizontal_datum_name'
         att = 'Monte Mario (Rome)'
-        res = self.cf.check_grid_mapping_attr_condition(att, att_name)
+        res = self.cf._check_grid_mapping_attr_condition(att, att_name)
         self.assertTrue(res[0])
 
         att_name = 'prime_meridian_name'
         att = 'Athens'
-        res = self.cf.check_grid_mapping_attr_condition(att, att_name)
+        res = self.cf._check_grid_mapping_attr_condition(att, att_name)
         self.assertTrue(res[0])
 
         att_name = 'reference_ellipsoid_name'
         att = 'Airy 1830'
-        res = self.cf.check_grid_mapping_attr_condition(att, att_name)
+        res = self.cf._check_grid_mapping_attr_condition(att, att_name)
         self.assertTrue(res[0])
 
         att_name = 'towgs84'
         att = np.array([0, 0, 0], dtype=np.float64) # len 3
-        res = self.cf.check_grid_mapping_attr_condition(att, att_name)
+        res = self.cf._check_grid_mapping_attr_condition(att, att_name)
         self.assertTrue(res[0])
-        
+
         att_name = 'towgs84'
         att = np.array([0, 0, 0, 0, 0, 0], dtype=np.float64) # len 6
-        res = self.cf.check_grid_mapping_attr_condition(att, att_name)
+        res = self.cf._check_grid_mapping_attr_condition(att, att_name)
         self.assertTrue(res[0])
 
         att_name = 'towgs84'
         att = np.array([0, 0, 0, 0, 0, 0, 0], dtype=np.float64) # len 7
-        res = self.cf.check_grid_mapping_attr_condition(att, att_name)
+        res = self.cf._check_grid_mapping_attr_condition(att, att_name)
         self.assertTrue(res[0])
 
         att_name = 'geographic_crs_name'
         att = 'NAD83(CSRS98)'
-        res = self.cf.check_grid_mapping_attr_condition(att, att_name)
+        res = self.cf._check_grid_mapping_attr_condition(att, att_name)
         self.assertTrue(res[0])
 
         att_name = 'geoid_name'
         att = 'Mayotte 1950'
-        res = self.cf.check_grid_mapping_attr_condition(att, att_name)
+        res = self.cf._check_grid_mapping_attr_condition(att, att_name)
         self.assertTrue(res[0])
 
         att_name = 'geopotential_datum_name'
         att = 'NAVD88'
-        res = self.cf.check_grid_mapping_attr_condition(att, att_name)
+        res = self.cf._check_grid_mapping_attr_condition(att, att_name)
         self.assertTrue(res[0])
 
         att_name = 'projected_crs_name'
         att = 'Anguilla 1957 / British West Indies Grid'
-        res = self.cf.check_grid_mapping_attr_condition(att, att_name)
+        res = self.cf._check_grid_mapping_attr_condition(att, att_name)
         self.assertTrue(res[0])
 
         # test bad
 
         att_name = 'horizontal_datum_name'
         att = 'bad'
-        res = self.cf.check_grid_mapping_attr_condition(att, att_name)
+        res = self.cf._check_grid_mapping_attr_condition(att, att_name)
         self.assertFalse(res[0])
 
         att_name = 'prime_meridian_name'
         att = 'bad'
-        res = self.cf.check_grid_mapping_attr_condition(att, att_name)
+        res = self.cf._check_grid_mapping_attr_condition(att, att_name)
         self.assertFalse(res[0])
 
         att_name = 'reference_ellipsoid_name'
         att = 'goofy goober'
-        res = self.cf.check_grid_mapping_attr_condition(att, att_name)
+        res = self.cf._check_grid_mapping_attr_condition(att, att_name)
         self.assertFalse(res[0])
 
         att_name = 'towgs84'
         att = np.array([0, 0, 0], dtype=np.int64) # len 3, wrong dtype
-        res = self.cf.check_grid_mapping_attr_condition(att, att_name)
+        res = self.cf._check_grid_mapping_attr_condition(att, att_name)
         self.assertFalse(res[0])
 
         att_name = 'towgs84'
         att = np.array([0, 0, 0, 0], dtype=np.int64) # len 4
-        res = self.cf.check_grid_mapping_attr_condition(att, att_name)
+        res = self.cf._check_grid_mapping_attr_condition(att, att_name)
         self.assertFalse(res[0])
 
         att_name = 'towgs84'
         att = np.float64(0) # single value, right dtype
-        res = self.cf.check_grid_mapping_attr_condition(att, att_name)
+        res = self.cf._check_grid_mapping_attr_condition(att, att_name)
         self.assertFalse(res[0])
 
         att_name = 'geographic_crs_name'
         att = 'badbadbadbadbadnotinhere'
-        res = self.cf.check_grid_mapping_attr_condition(att, att_name)
+        res = self.cf._check_grid_mapping_attr_condition(att, att_name)
         self.assertFalse(res[0])
 
         att_name = 'geoid_name'
         att = 'yooooooo'
-        res = self.cf.check_grid_mapping_attr_condition(att, att_name)
+        res = self.cf._check_grid_mapping_attr_condition(att, att_name)
         self.assertFalse(res[0])
 
         att_name = 'geopotential_datum_name'
         att = 'NAVBAD BAD'
-        res = self.cf.check_grid_mapping_attr_condition(att, att_name)
+        res = self.cf._check_grid_mapping_attr_condition(att, att_name)
         self.assertFalse(res[0])
 
         att_name = 'projected_crs_name'
         att = 'Teddy Bruschi'
-        res = self.cf.check_grid_mapping_attr_condition(att, att_name)
+        res = self.cf._check_grid_mapping_attr_condition(att, att_name)
         self.assertFalse(res[0])