Repository: ioos/compliance-checker
Pull request #503 (2017-07-22 01:23:41)

Fixes cf_role checks
The cf_role checks was returning an empty set without any actual checks.
This patch fixes the check for cf_role and addresses #491.


Patch:
diff --git a/compliance_checker/cf/cf.py b/compliance_checker/cf/cf.py
index a0892f7..56f8932 100644
--- a/compliance_checker/cf/cf.py
+++ b/compliance_checker/cf/cf.py
@@ -3097,16 +3097,21 @@ class CFBaseCheck(BaseCheck):
         :param netCDF4.Dataset ds: An open netCDF dataset
         :rtype: compliance_checker.base.Result
         """
-        ret_val = []
         valid_roles = ['timeseries_id', 'profile_id', 'trajectory_id']
+        variable_count = 0
         for variable in ds.get_variables_by_attributes(cf_role=lambda x: x is not None):
+            variable_count += 1
             name = variable.name
             valid_cf_role = TestCtx(BaseCheck.HIGH, '§9.5 {} contains a valid cf_role attribute'.format(name))
             cf_role = variable.cf_role
             valid_cf_role.assert_true(cf_role in valid_roles,
                                       "{} is not a valid cf_role value. It must be one of {}"
                                       "".format(name, ', '.join(valid_roles)))
-        return ret_val
+        valid_cf_role.assert_true(variable_count < 3,
+                                  ('§9.5 states that datasets should not '
+                                   'contain more than two variables defining a '
+                                   'cf_role attribute.'))
+        return valid_cf_role.to_result()
 
     def check_variable_features(self, ds):
         '''


Test patch:
diff --git a/compliance_checker/tests/data/bad_cf_role.cdl b/compliance_checker/tests/data/bad_cf_role.cdl
new file mode 100644
index 0000000..a344d08
--- /dev/null
+++ b/compliance_checker/tests/data/bad_cf_role.cdl
@@ -0,0 +1,32 @@
+netcdf 3d-static-grid {
+dimensions:
+    station_id = 1;
+    profile_id = 1;
+    trajectory_id = 1;
+    lat = 2;
+    lon = 2;
+    depth = 6;
+variables:
+    char station(station_id);
+        station:cf_role = "timeseries_id";
+    char profile(profile_id);
+        profile:cf_role = "profile_id";
+    char trajectory(trajectory_id);
+        trajectory:cf_role = "trajectory_id";
+    double depth(station_id, profile_id);
+        depth:standard_name = "depth";
+        depth:positive = "down";
+        depth:units = "m";
+    double lat(station_id);
+        lat:standard_name = "latitude";
+        lat:units = "degrees_north";
+    double lon(station_id);
+        lon:standard_name = "longitude";
+        lon:units = "degrees_east";
+    double T(depth, lat, lon);
+        T:standard_name = "sea_water_temperature";
+        T:units = "deg_C";
+}
+
+
+
diff --git a/compliance_checker/tests/resources.py b/compliance_checker/tests/resources.py
index 5c47f5a..84735e4 100644
--- a/compliance_checker/tests/resources.py
+++ b/compliance_checker/tests/resources.py
@@ -111,4 +111,5 @@ STATIC_FILES = {
     'cell_measure'                         : get_filename('tests/data/cell_measure.cdl'),
     'bad_cell_measure1'                    : get_filename('tests/data/bad_cell_measure1.cdl'),
     'bad_cell_measure2'                    : get_filename('tests/data/bad_cell_measure2.cdl'),
+    'bad_cf_role'                          : get_filename('tests/data/bad_cf_role.cdl'),
 }
diff --git a/compliance_checker/tests/test_cf_integration.py b/compliance_checker/tests/test_cf_integration.py
index ea150c3..92271be 100644
--- a/compliance_checker/tests/test_cf_integration.py
+++ b/compliance_checker/tests/test_cf_integration.py
@@ -75,7 +75,7 @@ class TestCFIntegration(BaseTestCase):
         dataset = self.load_dataset(STATIC_FILES['sldmb_43093_agg'])
         check_results = self.cs.run(dataset, [], 'cf')
         scored, out_of, messages = self.get_results(check_results)
-        assert (scored, out_of) == (137, 145)
+        assert (scored, out_of) == (139, 147)
         assert len(messages) == 8
         assert u'standard_name temperature is not defined in Standard Name Table v36' in messages
         assert (u'auxiliary coordinate specified by the coordinates attribute, precise_lat, '
@@ -101,7 +101,7 @@ class TestCFIntegration(BaseTestCase):
         dataset = self.load_dataset(STATIC_FILES['l01-met'])
         check_results = self.cs.run(dataset, [], 'cf')
         scored, out_of, messages = self.get_results(check_results)
-        assert (scored, out_of) == (588, 602)
+        assert (scored, out_of) == (590, 604)
         assert len(messages) == 16
 
         # The variable is supposed to be a status flag but it's mislabled
@@ -123,8 +123,8 @@ class TestCFIntegration(BaseTestCase):
         dataset = self.load_dataset(STATIC_FILES['sp041'])
         check_results = self.cs.run(dataset, [], 'cf')
         scored, out_of, messages = self.get_results(check_results)
-        assert (scored, out_of) == (1188, 1192)
-        assert len(messages) == 4
+        assert (scored, out_of) == (1189, 1194)
+        assert len(messages) == 5
 
         assert (u"lat_qc is not a variable in this dataset") in messages
         assert (u"TrajectoryProfile is not a valid CF featureType. It must be one of point, "
@@ -139,7 +139,7 @@ class TestCFIntegration(BaseTestCase):
         dataset = self.load_dataset(STATIC_FILES['3mf07'])
         check_results = self.cs.run(dataset, [], 'cf')
         scored, out_of, messages = self.get_results(check_results)
-        assert (scored, out_of) == (418, 426)
+        assert (scored, out_of) == (420, 428)
         assert len(messages) == 10
         assert (u"dimensions for auxiliary coordinate variable z (z) are not a subset of dimensions for "
                 "variable flag (profile)") in messages
@@ -155,7 +155,7 @@ class TestCFIntegration(BaseTestCase):
         dataset = self.load_dataset(STATIC_FILES['ooi_glider'])
         check_results = self.cs.run(dataset, [], 'cf')
         scored, out_of, messages = self.get_results(check_results)
-        assert (scored, out_of) == (593, 597)
+        assert (scored, out_of) == (595, 599)
         assert len(messages) == 4
         assert (u"variable deployment's attribute standard_name must be a non-empty string or "
                 "it should define a long_name attribute.") in messages
@@ -182,7 +182,7 @@ class TestCFIntegration(BaseTestCase):
         dataset = self.load_dataset(STATIC_FILES['kibesillah'])
         check_results = self.cs.run(dataset, [], 'cf')
         scored, out_of, messages = self.get_results(check_results)
-        assert (scored, out_of) == (197, 200)
+        assert (scored, out_of) == (199, 202)
         assert len(messages) == 3
         assert (u"source should be defined") in messages
         assert (u"references should be defined") in messages
@@ -210,8 +210,8 @@ class TestCFIntegration(BaseTestCase):
         dataset = self.load_dataset(STATIC_FILES['fvcom'])
         check_results = self.cs.run(dataset, [], 'cf')
         scored, out_of, messages = self.get_results(check_results)
-        assert (scored, out_of) == (646, 651)
-        assert len(messages) == 24
+        assert (scored, out_of) == (646, 653)
+        assert len(messages) == 26
 
         for msg in messages:
             if msg.startswith("dimensions for auxiliary coordinate variable siglay"):
@@ -263,4 +263,16 @@ class TestCFIntegration(BaseTestCase):
         dataset = self.load_dataset(STATIC_FILES['NCEI_profile_template_v2_0'])
         check_results = self.cs.run(dataset, [], 'cf')
         scored, out_of, messages = self.get_results(check_results)
-        assert (scored, out_of) == (342, 346)
+        assert (scored, out_of) == (344, 348)
+
+    def test_bad_cf_roles(self):
+        '''
+        Tests the CF checker detects datasets with more than 2 defined cf_role variables
+        '''
+        dataset = self.load_dataset(STATIC_FILES['bad_cf_role'])
+        check_results = self.cs.run(dataset, [], 'cf')
+        scored, out_of, messages = self.get_results(check_results)
+        assert (scored, out_of) == (92, 100)
+        assert ('§9.5 states that datasets should not contain more than two '
+                'variables defining a cf_role attribute.') in messages
+