Repository: ioos/compliance-checker
Pull request #621 (2019-01-22 19:37:47)

Changes regex, adds climatology cell_methods tests
Changes the regex used to validate the climatology cell_methods to one
slightly shorter, and hopefully more efficient.  Also adds test
coverage for various valid and invalid climatology cell_methods
attributes.


Patch:
diff --git a/compliance_checker/cf/cf.py b/compliance_checker/cf/cf.py
index d1eff13..834681c 100644
--- a/compliance_checker/cf/cf.py
+++ b/compliance_checker/cf/cf.py
@@ -826,7 +826,7 @@ class CFBaseCheck(BaseCheck):
         for attr in attrs:
             dataset_attr = getattr(ds, attr, None)
             is_string = isinstance(dataset_attr, basestring)
-            valid_globals.assert_true(is_string and len(dataset_attr), 
+            valid_globals.assert_true(is_string and len(dataset_attr),
                                       "§2.6.2 global attribute {} should exist and be a non-empty string" # subsection message
                                       "".format(attr))
         return valid_globals.to_result()
@@ -2793,7 +2793,6 @@ class CFBaseCheck(BaseCheck):
         Checks that the spacing and/or comment info contained inside the
         parentheses in cell_methods is well-formed
         """
-
         valid_info = TestCtx(BaseCheck.MEDIUM,
                              self.section_titles['7.3'])
         # if there are no colons, this is a simple comment
@@ -2952,8 +2951,6 @@ class CFBaseCheck(BaseCheck):
                     u"Climatology dimension {} should only contain two elements".format(
                         boundary_variable.dimensions)
                 )
-        # catchall
-        return ret_val # TODO this return cuts right in middle of this method
 
         # otherwise match the following values with for variable with
         # `cell_methods` attributes
@@ -2965,9 +2962,10 @@ class CFBaseCheck(BaseCheck):
         # "time: method1 within years time: method2 over years (sidereal years)"
 
         meth_regex = "(?:{})".format("|".join(methods))
-        re_string = (r"^time: {0} within (?:years time: {0} over years|"
-                     r"days time: {0} over days"
-                     r"(?: time: {0} over years)?)(?: \([^)]+\))?$".format(meth_regex))
+        re_string = (r'^time: {0} within (years|days)'
+                     r' time: {0} over \1(?<=days)(?: time: {0} over years)?'
+                     r'(?: \([^)]+\))?$'.format(meth_regex))
+
         # find any variables with a valid climatological cell_methods
         for cell_method_var in ds.get_variables_by_attributes(cell_methods=lambda s: s is not
                                                               None):


Test patch:
diff --git a/compliance_checker/tests/test_cf.py b/compliance_checker/tests/test_cf.py
index 6133d24..e185a29 100644
--- a/compliance_checker/tests/test_cf.py
+++ b/compliance_checker/tests/test_cf.py
@@ -345,10 +345,43 @@ class TestCF(BaseTestCase):
         message = u'Cell measure variable PS referred to by box_area is not present in dataset variables'
         assert message in messages
 
-    def test_climatology(self):
+    def test_climatology_cell_methods(self):
+        """
+        Checks that climatology cell_methods strings are properly validated
+        """
         dataset = self.load_dataset(STATIC_FILES['climatology'])
         results = self.cf.check_climatological_statistics(dataset)
+        # cell methods in this file is
+        # "time: mean within days time: mean over days"
+        score, out_of, messages = self.get_results(results)
+        self.assertEqual(score, out_of)
+        temp_var = dataset.variables['temperature'] = \
+                   MockVariable(dataset.variables['temperature'])
+        temp_var.cell_methods = 'INVALID'
+        results = self.cf.check_climatological_statistics(dataset)
+        score, out_of, messages = self.get_results(results)
+        self.assertNotEqual(score, out_of)
+        # incorrect time units
+        temp_var.cell_methods = "time: mean within years time: mean over days"
+        results = self.cf.check_climatological_statistics(dataset)
+        score, out_of, messages = self.get_results(results)
+        self.assertNotEqual(score, out_of)
+        # can only have third method over years if first two are within and
+        # over days, respectively
+        temp_var.cell_methods = "time: mean within years time: mean over years time: sum over years"
+        results = self.cf.check_climatological_statistics(dataset)
+        score, out_of, messages = self.get_results(results)
+        self.assertNotEqual(score, out_of)
+        # this, on the other hand, should work.
+        temp_var.cell_methods = "time: mean within days time: mean over days time: sum over years"
+        results = self.cf.check_climatological_statistics(dataset)
+        score, out_of, messages = self.get_results(results)
+        self.assertEqual(score, out_of)
+        # parenthesized comment to describe climatology
+        temp_var.cell_methods = "time: sum within days time: maximum over days (ENSO years)"
+        results = self.cf.check_climatological_statistics(dataset)
         score, out_of, messages = self.get_results(results)
+        self.assertEqual(score, out_of)
 
     def test_check_ancillary_variables(self):
         '''