Repository: ioos/compliance-checker
Pull request #787 (2020-03-30 18:59:32)

Ioos convert to conditional results
Only use messages if the check fails.


Patch:
diff --git a/compliance_checker/base.py b/compliance_checker/base.py
index da21fa8..0928fb8 100644
--- a/compliance_checker/base.py
+++ b/compliance_checker/base.py
@@ -465,7 +465,7 @@ def attr_check(kvp, ds, priority, ret_val, gname=None, var_name=None):
         else:
             check_val = attr_result[1]
             res_tup = other.validate(name, check_val)
-        msgs = [] if res_tup[1] is None else res_tup[1]
+            msgs = [] if res_tup[1] is None else [res_tup[1]]
         ret_val.append(
             Result(
                 priority,
diff --git a/compliance_checker/ioos.py b/compliance_checker/ioos.py
index 15bd396..ae73148 100644
--- a/compliance_checker/ioos.py
+++ b/compliance_checker/ioos.py
@@ -420,7 +420,7 @@ class IOOS1_2Check(IOOSNCCheck):
             ("standard_name_url", BaseCheck.MEDIUM),
             #( "platform", BaseCheck.HIGH) # checked under check_single_platform()
             #( "wmo_platform_code", BaseCheck.HIGH) # only "if applicable", see check_wmo_platform_code()
-            #( "ancillary_variables", BaseCheck.HIGH) # only "if applicable", see _check_var_gts_ingest() 
+            #( "ancillary_variables", BaseCheck.HIGH) # only "if applicable", see _check_var_gts_ingest()
         ]))
 
         # geospatial vars must have the following attrs:
@@ -569,7 +569,7 @@ class IOOS1_2Check(IOOSNCCheck):
             not isinstance(igst, str) and igst is not None):
             r = False
 
-        return Result(BaseCheck.MEDIUM, r, "ioos_ingest", [m])
+        return Result(BaseCheck.MEDIUM, r, "ioos_ingest", None if r else [m])
 
     def check_contributor_role_and_vocabulary(self, ds):
         """
@@ -600,26 +600,26 @@ class IOOS1_2Check(IOOSNCCheck):
 
         if role:
             if role in [ # each in both vocabularies
-                "author",               
-                "coAuthor",             
-                "collaborator",         
-                "contributor",          
-                "custodian",            
-                "distributor",          
-                "editor",               
-                "funder",               
-                "mediator",             
-                "originator",           
-                "owner",                
-                "pointOfContact",       
+                "author",
+                "coAuthor",
+                "collaborator",
+                "contributor",
+                "custodian",
+                "distributor",
+                "editor",
+                "funder",
+                "mediator",
+                "originator",
+                "owner",
+                "pointOfContact",
                 "principalInvestigator",
-                "processor",            
-                "publisher",            
-                "resourceProvider",     
-                "rightsHolder",         
-                "sponsor",              
-                "stakeholder",          
-                "user"                  
+                "processor",
+                "publisher",
+                "resourceProvider",
+                "rightsHolder",
+                "sponsor",
+                "stakeholder",
+                "user"
             ]:
                 role_val = True
 
@@ -631,8 +631,8 @@ class IOOS1_2Check(IOOSNCCheck):
                 vocb_val = True
 
         return [
-            Result(BaseCheck.MEDIUM, role_val, "contributor_role", [role_msg]),
-            Result(BaseCheck.MEDIUM, vocb_val, "contributor_role_vocabulary", [vocb_msg]),
+            Result(BaseCheck.MEDIUM, role_val, "contributor_role", None if role_val else [role_msg]),
+            Result(BaseCheck.MEDIUM, vocb_val, "contributor_role_vocabulary", None if vocb_val else [vocb_msg]),
         ]
 
     def check_geophysical_vars_have_attrs(self, ds):
@@ -781,12 +781,12 @@ class IOOS1_2Check(IOOSNCCheck):
             if re.match(r'^\S+$', p):
                 r = True
 
-        return Result(BaseCheck.HIGH, r, "platform", [m.format(p)])
+        return Result(BaseCheck.HIGH, r, "platform", None if r else [m.format(p)])
 
     def check_single_platform(self, ds):
         """
         Verify that a dataset only has a single platform attribute. If one exists,
-        examine the featureType of the dataset. If the featureType is 
+        examine the featureType of the dataset. If the featureType is
         [point, timeSeries, profile, trajectory] and cf_role in [timeseries_id,
         profile_id, trajectory_id] dimensionality of the variable containing
         cf_role must be 1 as "we only want a single glider/auv/ship"; if
@@ -815,23 +815,23 @@ class IOOS1_2Check(IOOSNCCheck):
         if num_platforms > 1 and glb_platform:
             msg = "A dataset may only have one platform; {} found".format(len(platform_set))
             val = False
-            results.append(Result(BaseCheck.HIGH, val, "platform", [msg]))
+            results.append(Result(BaseCheck.HIGH, val, "platform", None if val else [msg]))
 
 
         elif ((not glb_platform) and num_platforms > 0):
             msg = "If platform variables exist, a global attribute \"platform\" must also exist"
             val = False
-            results.append(Result(BaseCheck.HIGH, val, "platform", [msg]))
+            results.append(Result(BaseCheck.HIGH, val, "platform", None if val else [msg]))
 
         elif num_platforms == 0 and glb_platform:
             msg = "A dataset with a global \"platform\" attribute must platform have variables"
             val = False
-            results.append(Result(BaseCheck.HIGH, val, "platform", [msg]))
+            results.append(Result(BaseCheck.HIGH, val, "platform", None if val else [msg]))
 
         elif num_platforms == 0 and (not glb_platform):
             msg = "Gridded model datasets are not required to declare a platform"
             val = True
-            results.append(Result(BaseCheck.HIGH, val, "platform", [msg]))
+            results.append(Result(BaseCheck.HIGH, val, "platform", None if val else [msg]))
 
         else: # num_platforms==1 and glb_platform, test the dimensionality
 
@@ -868,7 +868,7 @@ class IOOS1_2Check(IOOSNCCheck):
                            BaseCheck.HIGH,
                            _val,
                            "platform variables",
-                           [msg.format(cf_role_var=var.name, cf_role=cf_role, dim=shp)]
+                           None if _val else [msg.format(cf_role_var=var.name, cf_role=cf_role, dim=shp)]
                        )
                     )
 
@@ -891,8 +891,8 @@ class IOOS1_2Check(IOOSNCCheck):
         m = "platform_vocabulary must be a valid URL"
         pvocab = getattr(ds, "platform_vocabulary", "")
         val = bool(validators.url(pvocab))
-        return Result(BaseCheck.MEDIUM, val, "platform_vocabulary", [m])
-            
+        return Result(BaseCheck.MEDIUM, val, "platform_vocabulary", None if val else [m])
+
     def _check_gts_ingest_val(self, val):
         """
         Check that `val` is a str and is equal to "true" or "false"
@@ -923,13 +923,15 @@ class IOOS1_2Check(IOOSNCCheck):
         Result
         """
 
-        r = True # default
-        gts = getattr(ds, "gts_ingest", None)
+        gts_ingest_value = getattr(ds, "gts_ingest", None)
 
-        if gts:
-            r = self._check_gts_ingest_val(gts)
+        is_valid_string = True
+        if isinstance(gts_ingest_value, str):
+            is_valid_string = self._check_gts_ingest_val(gts_ingest_value)
 
-        return Result(BaseCheck.HIGH, r, "gts_ingest", ["gts_ingest must be a string \"true\" or \"false\""])
+        fail_message = ["gts_ingest must be a string \"true\" or \"false\""]
+        return Result(BaseCheck.HIGH, is_valid_string, "gts_ingest",
+                      None if is_valid_string else fail_message)
 
     def _var_qualifies_for_gts_ingest(self, ds, var):
         """
@@ -1013,34 +1015,40 @@ class IOOS1_2Check(IOOSNCCheck):
 
         # check variables
         all_passed_ingest_reqs = True # default
+        var_failed_ingest_msg = None
+        var_passed_ingest_msg = None
 
         if do_gts: # execute if dataset flagged for ingest
 
             var_passed_ingest_reqs = set()
-            var_passed_ingest_msg  = "The following variables qualifed for GTS Ingest:"
-            var_failed_ingest_msg  = "The following variables did not qualify for GTS Ingest:"
-
             for v in ds.get_variables_by_attributes(gts_ingest=lambda x: x=="true"):
                 var_passed_ingest_reqs.add((v.name, self._var_qualifies_for_gts_ingest(ds, v)))
 
-            for var_name, pass_result in var_passed_ingest_reqs:
-                if pass_result:
-                    var_passed_ingest_msg += "\n - {}".format(var_name)
-                else:
-                    var_failed_ingest_msg += "\n - {}".format(var_name)
-                    all_passed_ingest_reqs = False
-    
-            # join messages together
-            var_passed_ingest_msg += "\n{}".format(var_failed_ingest_msg)
+            all_passed_ingest_reqs = all(map(lambda x: x[1], var_passed_ingest_reqs))
+            if not all_passed_ingest_reqs:
+                _var_failed = map(lambda y: y[0], filter(lambda x: not x[1], var_passed_ingest_reqs))
+                _var_passed = map(lambda y: y[0], filter(lambda x: x[1], var_passed_ingest_reqs))
+
+                var_failed_ingest_msg = (
+                                            "The following variables did not "
+                                            "qualify for GTS Ingest: {}".format(
+                                                ", ".join(_var_failed),
+                                            )
+                                        )
+
+                var_passed_ingest_msg = (
+                                            "The following variables qualified "
+                                            "for GTS Ingest: {}\n".format(
+                                                  ", ".join(_var_passed)
+                                            )
+                                        )
 
-        else:
-            var_passed_ingest_msg = "Dataset not flagged for GTS Ingest; skipping variable checks"
 
         return Result(
             BaseCheck.HIGH,
             all_passed_ingest_reqs,
             "gts_ingest requirements",
-            [var_passed_ingest_msg]
+            [var_passed_ingest_msg, var_failed_ingest_msg]
         )
 
     def check_instrument_variables(self, ds):
@@ -1148,7 +1156,7 @@ class IOOS1_2Check(IOOSNCCheck):
                 msg = "\"references\" attribute for variable \"{}\" must be a valid URL".format(v.name)
                 val = bool(validators.url(attval))
 
-            results.append(Result(BaseCheck.MEDIUM, val, "qartod_variable:references", [msg]))
+            results.append(Result(BaseCheck.MEDIUM, val, "qartod_variable:references", None if val else [msg]))
 
         return results
 
@@ -1187,7 +1195,7 @@ class IOOS1_2Check(IOOSNCCheck):
            ):
                valid = False
 
-        return Result(BaseCheck.HIGH, valid, ctxt, [msg])
+        return Result(BaseCheck.HIGH, valid, ctxt, None if valid else [msg])
 
 class IOOSBaseSOSCheck(BaseCheck):
     _cc_spec = 'ioos_sos'


Test patch:
diff --git a/compliance_checker/tests/test_ioos_profile.py b/compliance_checker/tests/test_ioos_profile.py
index 0eaf053..3706916 100644
--- a/compliance_checker/tests/test_ioos_profile.py
+++ b/compliance_checker/tests/test_ioos_profile.py
@@ -370,6 +370,7 @@ class TestIOOS1_2(BaseTestCase):
         ds.setncattr("contributor_role", "contributor")
         results = self.ioos.check_contributor_role_and_vocabulary(ds)
         self.assertTrue(results[0].value)
+        self.assertEqual(results[0].msgs, [])
         self.assertFalse(results[1].value)
 
         # bad role, good vocab
@@ -378,19 +379,24 @@ class TestIOOS1_2(BaseTestCase):
         results = self.ioos.check_contributor_role_and_vocabulary(ds)
         self.assertFalse(results[0].value)
         self.assertTrue(results[1].value)
+        self.assertEqual(results[1].msgs, [])
         
         # good role, good vocab
         ds.setncattr("contributor_role", "contributor")
         ds.setncattr("contributor_role_vocabulary", "http://vocab.nerc.ac.uk/collection/G04/current/")
         results = self.ioos.check_contributor_role_and_vocabulary(ds)
         self.assertTrue(results[0].value)
+        self.assertEqual(results[0].msgs, [])
         self.assertTrue(results[1].value)
+        self.assertEqual(results[1].msgs, [])
 
         ds.setncattr("contributor_role", "resourceProvider")
         ds.setncattr("contributor_role_vocabulary", "https://www.ngdc.noaa.gov/wiki/index.php?title=ISO_19115_and_19115-2_CodeList_Dictionaries#CI_RoleCode")
         results = self.ioos.check_contributor_role_and_vocabulary(ds)
         self.assertTrue(results[0].value)
+        self.assertEqual(results[0].msgs, [])
         self.assertTrue(results[1].value)
+        self.assertEqual(results[1].msgs, [])
 
     def test_check_creator_and_publisher_type(self):
         """
@@ -426,11 +432,13 @@ class TestIOOS1_2(BaseTestCase):
         # no gts_ingest_requirements, should pass
         result = self.ioos.check_gts_ingest_global(ds)
         self.assertTrue(result.value)
+        self.assertEqual(result.msgs, [])
 
         # passing value
         ds.setncattr("gts_ingest", "true")
         result = self.ioos.check_gts_ingest_global(ds)
         self.assertTrue(result.value)
+        self.assertEqual(result.msgs, [])
 
         ds.setncattr("gts_ingest", "false")
         result = self.ioos.check_gts_ingest_global(ds)
@@ -525,6 +533,7 @@ class TestIOOS1_2(BaseTestCase):
         # no wmo_platform_code, pass
         result = self.ioos.check_wmo_platform_code(ds)
         self.assertTrue(result.value)
+        self.assertEqual(result.msgs, [])
 
         # valid code
         ds.setncattr("wmo_platform_code", "12345")
@@ -667,7 +676,9 @@ class TestIOOS1_2(BaseTestCase):
 
         # good value
         ds.setncattr("platform", "single_string")
-        self.assertTrue(self.ioos.check_platform_global(ds).value)
+        res = self.ioos.check_platform_global(ds)
+        self.assertTrue(res.value)
+        self.assertEqual(res.msgs, [])
 
     def test_check_single_platform(self):
 
@@ -676,6 +687,7 @@ class TestIOOS1_2(BaseTestCase):
         # no global attr but also no platform variables, should pass
         results = self.ioos.check_single_platform(ds)
         self.assertTrue(results[0].value)
+        self.assertEqual(results[0].msgs, [])
 
         # give platform global, no variables, fail
         ds.setncattr("platform", "buoy")
@@ -692,6 +704,7 @@ class TestIOOS1_2(BaseTestCase):
         cf_role_var.setncattr("cf_role", "timeseries_id")
         results = self.ioos.check_single_platform(ds)
         self.assertTrue(all(r.value for r in results))
+        self.assertTrue(all(r.msgs==[] for r in results))
 
         # global attr, multiple platform variables, correct cf_role & featureType, fail
         plat2 = ds.createVariable("platform_var_2", np.byte)
@@ -719,7 +732,9 @@ class TestIOOS1_2(BaseTestCase):
     def test_check_platform_vocabulary(self):
         ds = MockTimeSeries() # time, lat, lon, depth 
         ds.setncattr("platform_vocabulary", "http://google.com")
-        self.assertTrue(self.ioos.check_platform_vocabulary(ds).value)
+        result = self.ioos.check_platform_vocabulary(ds)
+        self.assertTrue(result.value)
+        self.assertEqual(result.msgs, [])
 
         ds.setncattr("platform_vocabulary", "bad")
         self.assertFalse(self.ioos.check_platform_vocabulary(ds).value)
@@ -781,6 +796,7 @@ class TestIOOS1_2(BaseTestCase):
         qr.setncattr("references", "http://services.cormp.org/quality.php")
         results = self.ioos.check_qartod_variables_references(ds)
         self.assertTrue(all(r.value for r in results))
+        self.assertEqual(results[0].msgs, []) # only one Result to test
 
         # QARTOD variable with bad references (fail)
         qr.setncattr("references", r"p9q384ht09q38@@####???????////??//\/\/\/\//\/\74ht")
@@ -791,7 +807,9 @@ class TestIOOS1_2(BaseTestCase):
         ds = MockTimeSeries()
 
         # no value, pass
-        self.assertTrue(self.ioos.check_ioos_ingest(ds).value)
+        res = self.ioos.check_ioos_ingest(ds)
+        self.assertTrue(res.value)
+        self.assertEqual(res.msgs, [])
 
         # value false
         ds.setncattr("ioos_ingest", "false")