Repository: ioos/compliance-checker
Pull request #521 (2017-09-28 20:25:38)

Grid lat lon check fixes
Fix for #508 , which caused errors on certain rotated lon/lat variables.


Patch:
diff --git a/compliance_checker/cf/cf.py b/compliance_checker/cf/cf.py
index a3512f9..18e10e5 100644
--- a/compliance_checker/cf/cf.py
+++ b/compliance_checker/cf/cf.py
@@ -1521,10 +1521,14 @@ class CFBaseCheck(BaseCheck):
 
             # Check that latitude uses allowed units
             allowed_units = TestCtx(BaseCheck.MEDIUM, '§4.1 Latitude variable {} uses recommended units'.format(latitude))
-            if 'rotated_latitude_longitude' in grid_mapping and standard_name == 'grid_latitude':
-                allowed_units.assert_true(units == 'degrees',
-                                          "latitude variable '{}' should use degrees for units in rotated pole grid"
-                                          "".format(latitude))
+            if standard_name == 'grid_latitude':
+                e_n_units = cfutil.VALID_LAT_UNITS + cfutil.VALID_LON_UNITS
+                # check that the units aren't in east and north degrees units,
+                # but are convertible to angular units
+                allowed_units.assert_true(units not in e_n_units and
+                                          Unit(units) == Unit('degree'),
+                                          "Grid latitude variable '{}' should use degree equivalent units without east or north components. "
+                                          "Current units are {}".format(latitude, units))
             else:
                 allowed_units.assert_true(units_is_string and units.lower() in allowed_lat_units,
                                           "latitude variable '{}' should define valid units for latitude"
@@ -1611,10 +1615,14 @@ class CFBaseCheck(BaseCheck):
 
             # Check that longitude uses allowed units
             allowed_units = TestCtx(BaseCheck.MEDIUM, '§4.1 Longitude variable {} uses recommended units'.format(longitude))
-            if 'rotated_latitude_longitude' in grid_mapping and standard_name == 'grid_longitude':
-                allowed_units.assert_true(units == 'degrees',
-                                          "longitude variable '{}' should use degrees for units in rotated pole grid"
-                                          "".format(longitude))
+            if standard_name == 'grid_longitude':
+                e_n_units = cfutil.VALID_LAT_UNITS + cfutil.VALID_LON_UNITS
+                # check that the units aren't in east and north degrees units,
+                # but are convertible to angular units
+                allowed_units.assert_true(units not in e_n_units and
+                                          Unit(units) == Unit('degree'),
+                                          "Grid longitude variable '{}' should use degree equivalent units without east or north components. "
+                                          "Current units are {}".format(longitude, units))
             else:
                 allowed_units.assert_true(units_is_string and units.lower() in allowed_lon_units,
                                           "longitude variable '{}' should define valid units for longitude"
@@ -2151,7 +2159,7 @@ class CFBaseCheck(BaseCheck):
             # contents of the `coordinates` attribute only.
             axis_map = cfutil.get_axis_map(ds, variable)
 
-            # Make sure we can find latitude and it's dimensions are a subset
+            # Make sure we can find latitude and its dimensions are a subset
             found_lat = False
             for lat in axis_map['Y']:
                 is_subset_dims = set(ds.variables[lat].dimensions).issubset(dimensions)
@@ -2163,7 +2171,7 @@ class CFBaseCheck(BaseCheck):
                                    '{} is not associated with a coordinate defining true latitude '
                                    'and sharing a subset of dimensions'.format(variable))
 
-            # Make sure we can find longitude and it's dimensions are a subset
+            # Make sure we can find longitude and its dimensions are a subset
             found_lon = False
             for lon in axis_map['X']:
                 is_subset_dims = set(ds.variables[lon].dimensions).issubset(dimensions)
@@ -2288,7 +2296,7 @@ class CFBaseCheck(BaseCheck):
         ret_val = []
         grid_mapping_variables = cfutil.get_grid_mapping_variables(ds)
 
-        # Check the grid_mapping attribute to be a non-empty string and that it's reference exists
+        # Check the grid_mapping attribute to be a non-empty string and that its reference exists
         for variable in ds.get_variables_by_attributes(grid_mapping=lambda x: x is not None):
             grid_mapping = getattr(variable, 'grid_mapping', None)
             defines_grid_mapping = TestCtx(BaseCheck.HIGH,


Test patch:
diff --git a/compliance_checker/tests/helpers.py b/compliance_checker/tests/helpers.py
index 421bc34..e369b65 100644
--- a/compliance_checker/tests/helpers.py
+++ b/compliance_checker/tests/helpers.py
@@ -30,7 +30,12 @@ class MockTimeSeries(MockNetCDF):
 
 class MockVariable(object):
     '''
-    For mocking a dataset variable
+    For mocking a dataset variable. Constructor optionally takes a NetCDF
+    variable, the NetCDF attributes of which will be copied over to this
+    object.
     '''
-    pass
 
+    def __init__(self, copy_var=None):
+        if copy_var is not None:
+            for att in copy_var.ncattrs():
+                setattr(self, att, getattr(copy_var, att))
diff --git a/compliance_checker/tests/test_cf.py b/compliance_checker/tests/test_cf.py
index 00283c8..1844e40 100644
--- a/compliance_checker/tests/test_cf.py
+++ b/compliance_checker/tests/test_cf.py
@@ -546,6 +546,20 @@ class TestCF(BaseTestCase):
         results = self.cf.check_latitude(dataset)
         scored, out_of, messages = self.get_results(results)
         assert (scored, out_of) == (6, 6)
+        # hack to avoid writing to read-only file
+        dataset.variables['rlat'] = MockVariable(dataset.variables['rlat'])
+        rlat = dataset.variables['rlat']
+        rlat.name = 'rlat'
+        # test with a bad value
+        rlat.units = 'degrees_north'
+        results = self.cf.check_latitude(dataset)
+        scored, out_of, messages = self.get_results(results)
+        wrong_format = "Grid latitude variable '{}' should use degree equivalent units without east or north components. Current units are {}"
+        self.assertTrue(wrong_format.format(rlat.name, rlat.units) in messages)
+        rlat.units = 'radians'
+        results = self.cf.check_latitude(dataset)
+        scored, out_of, messages = self.get_results(results)
+        self.assertTrue(wrong_format.format(rlat.name, rlat.units) in messages)
 
     def test_longitude(self):
         '''
@@ -597,6 +611,20 @@ class TestCF(BaseTestCase):
         results = self.cf.check_latitude(dataset)
         scored, out_of, messages = self.get_results(results)
         assert (scored, out_of) == (6, 6)
+        # hack to avoid writing to read-only file
+        dataset.variables['rlon'] = MockVariable(dataset.variables['rlon'])
+        rlon = dataset.variables['rlon']
+        rlon.name = 'rlon'
+        # test with a bad value
+        rlon.units = 'degrees_east'
+        results = self.cf.check_longitude(dataset)
+        scored, out_of, messages = self.get_results(results)
+        wrong_format = "Grid longitude variable '{}' should use degree equivalent units without east or north components. Current units are {}"
+        self.assertTrue(wrong_format.format(rlon.name, rlon.units) in messages)
+        rlon.units = 'radians'
+        results = self.cf.check_longitude(dataset)
+        scored, out_of, messages = self.get_results(results)
+        self.assertTrue(wrong_format.format(rlon.name, rlon.units) in messages)
 
     def test_is_vertical_coordinate(self):
         '''