Repository: ioos/compliance-checker
Pull request #643 (2019-04-16 19:05:45)

ACDD refactor date
Alters priority of tests relating to ISO date global attributes in ACDD 1.3.

Refactors handling of global attribute lists to use an `OrderedDict` for the underlying operations.

Addresses #519


Patch:
diff --git a/compliance_checker/acdd.py b/compliance_checker/acdd.py
index ae2ac29..621cbb3 100644
--- a/compliance_checker/acdd.py
+++ b/compliance_checker/acdd.py
@@ -14,6 +14,8 @@ from compliance_checker.base import (BaseCheck, BaseNCCheck, check_has,
 from compliance_checker.cf.util import _possiblexunits, _possibleyunits
 from compliance_checker.util import datetime_is_iso, dateparse
 from compliance_checker import cfutil
+from compliance_checker.util import kvp_convert
+from functools import partial
 import pendulum
 from pygeoif import from_wkt
 
@@ -239,7 +241,7 @@ class ACDDBaseCheck(BaseCheck):
                           'geospatial_lat_extents_match',
                           ['Could not convert one of geospatial_lat_min ({}) or max ({}) to float see CF-1.6 spec chapter 4.1'
                           ''.format(ds.geospatial_lat_min, ds.geospatial_lat_max)])
-            
+
 
         # identify lat var(s) as per CF 4.1
         lat_vars = {}       # var -> number of criteria passed
@@ -634,6 +636,28 @@ class ACDD1_3Check(ACDDNCCheck):
             'publisher_institution',
         ])
 
+        # overrride the ISO date checks in
+        def _check_attr_is_iso_date(attr, ds):
+            result_name = "{}_is_iso".format(attr)
+            if not hasattr(ds, attr):
+                return ratable_result((0,2), result_name,
+                                      ["Attr {} is not present".format(attr)])
+            else:
+                iso_check, msgs = datetime_is_iso(getattr(ds, attr))
+                return ratable_result((1 + iso_check, 2),
+                                      result_name, msgs)
+
+        # run ISO 8601 date checks against the date_created, date_issued,
+        # date_modified, and date_metadata_modified global attributes
+        self.rec_atts = kvp_convert(self.rec_atts)
+        self.rec_atts['date_created'] = partial(_check_attr_is_iso_date,
+                                                'date_created')
+        self.sug_atts = kvp_convert(self.sug_atts)
+        for k in ("date_{}".format(suffix) for suffix in
+                  ('issued', 'modified', 'metadata_modified')):
+
+            self.sug_atts[k] = partial(_check_attr_is_iso_date, k)
+
     def check_metadata_link(self, ds):
         '''
         Checks if metadata link is formed in a rational manner
@@ -649,39 +673,6 @@ class ACDD1_3Check(ACDDNCCheck):
         valid_link = (len(msgs) == 0)
         return Result(BaseCheck.LOW, valid_link, 'metadata_link_valid', msgs)
 
-    def check_date_modified_is_iso(self, ds):
-        '''
-        Checks if date modified field is ISO compliant
-
-        :param netCDF4.Dataset ds: An open netCDF dataset
-        '''
-        if not hasattr(ds, u'date_modified'):
-            return
-        date_modified_check, msgs = datetime_is_iso(getattr(ds, u'date_modified'))
-        return Result(BaseCheck.MEDIUM, date_modified_check, 'date_modified_is_iso', msgs)
-
-    def check_date_issued_is_iso(self, ds):
-        '''
-        Checks if date issued field is ISO compliant
-
-        :param netCDF4.Dataset ds: An open netCDF dataset
-        '''
-        if not hasattr(ds, u'date_issued'):
-            return
-        date_issued_check, msgs = datetime_is_iso(getattr(ds, u'date_issued'))
-        return Result(BaseCheck.MEDIUM, date_issued_check, 'date_issued_is_iso', msgs)
-
-    def check_date_metadata_modified_is_iso(self, ds):
-        '''
-        Checks if date metadata modified field is ISO compliant
-
-        :param netCDF4.Dataset ds: An open netCDF dataset
-        '''
-        if not hasattr(ds, u'date_metadata_modified'):
-            return
-        date_metadata_modified_check, msgs = datetime_is_iso(getattr(ds, u'date_metadata_modified'))
-        return Result(BaseCheck.MEDIUM, date_metadata_modified_check, 'date_metadata_modified_is_iso', msgs)
-
     def check_id_has_no_blanks(self, ds):
         '''
         Check if there are blanks in the id field
@@ -696,18 +687,6 @@ class ACDD1_3Check(ACDDNCCheck):
         else:
             return Result(BaseCheck.MEDIUM, True, 'no_blanks_in_id', msgs=[])
 
-    def check_date_created(self, ds):
-        '''
-        Check if date created is ISO-8601
-
-        :param netCDF4.Dataset ds: An open netCDF dataset
-        '''
-        if not hasattr(ds, u'date_created'):
-            return
-        date_created_check, msgs = datetime_is_iso(getattr(ds, u'date_created'))
-        return Result(BaseCheck.MEDIUM, date_created_check,
-                      'date_created_is_iso', msgs)
-
     def check_var_coverage_content_type(self, ds):
         '''
         Check coverage content type against valid ISO-19115-1 codes
diff --git a/compliance_checker/base.py b/compliance_checker/base.py
index 31cb2d2..ab10ca7 100644
--- a/compliance_checker/base.py
+++ b/compliance_checker/base.py
@@ -12,6 +12,7 @@ from owslib.swe.observation.sos100 import SensorObservationService_1_0_0
 from owslib.swe.sensor.sml import SensorML
 from owslib.namespaces import Namespaces
 from compliance_checker import __version__, MemoizedDataset
+from compliance_checker.util import kvp_convert
 from lxml import etree
 import sys
 
@@ -231,7 +232,7 @@ def xpath_check(tree, xpath):
     return len(xpath(tree)) > 0
 
 
-def attr_check(l, ds, priority, ret_val, gname=None):
+def attr_check(kvp, ds, priority, ret_val, gname=None):
     """
     Handles attribute checks for simple presence of an attribute, presence of
     one of several attributes, and passing a validation function.  Returns a
@@ -246,81 +247,19 @@ def attr_check(l, ds, priority, ret_val, gname=None):
     """
 
     msgs = []
-    if isinstance(l, tuple):
-        name, other = l
-        if hasattr(other, '__iter__'):
-            # redundant, we could easily do this with a hasattr
-            # check instead
-            res = std_check_in(ds, name, other)
-            if res == 0:
-                msgs.append("%s not present" % name)
-            elif res == 1:
-                msgs.append("%s present, but not in expected value list (%s)" % (name, other))
-
-            ret_val.append(
-                Result(
-                    priority,
-                    (res, 2),
-                    gname if gname else name, # groups Globals if supplied
-                    msgs
-                )
-            )
-        # if we have an XPath expression, call it on the document
-        elif type(other) is etree.XPath:
-            # TODO: store tree instead of creating it each time?
-            res = xpath_check(ds._root, other)
-            if not res:
-                msgs = ["XPath for {} not found".format(name)]
-            ret_val.append(
-                Result(
-                    priority,
-                    res,
-                    gname if gname else name,
-                    msgs
-                )
-            )
-        # if the attribute is a function, call it
-        # right now only supports single attribute
-        # important note: current magic approach uses all functions
-        # starting with "check".  Avoid naming check functions
-        # starting with check if you want to pass them in with
-        # a tuple to avoid them being checked more than once
-        elif hasattr(other, '__call__'):
-            # check that the attribute is actually present.
-            # This reduces boilerplate in functions by not needing
-            # to check whether the attribute is present every time
-            # and instead focuses on the core functionality of the
-            # test
-
-            res = other(ds) # call the method on the dataset
-            if not res:
-                msgs = ["%s not present" % name]
-                ret_val.append(
-                    Result(
-                        priority,
-                        res,
-                        gname if gname else name,
-                        msgs
-                    )
-                )
-            else:
-                ret_val.append(other(ds)(priority))
-        # unsupported second type in second
-        else:
-            raise TypeError("Second arg in tuple has unsupported type: {}".format(type(other)))
-
-    else:
-        res = std_check(ds, l)
+    name, other = kvp
+    if other is None:
+        res = std_check(ds, name)
         if not res:
-            msgs = ["%s not present" % l]
+            msgs = ["%s not present" % name]
         else:
             try:
                 # see if this attribute is a string, try stripping
                 # whitespace, and return an error if empty
-                att_strip = getattr(ds, l).strip()
+                att_strip = getattr(ds, name).strip()
                 if not att_strip:
                     res = False
-                    msgs = ["%s is empty or completely whitespace" % l]
+                    msgs = ["%s is empty or completely whitespace" % name]
             # if not a string/has no strip method we should be OK
             except AttributeError:
                 pass
@@ -329,9 +268,70 @@ def attr_check(l, ds, priority, ret_val, gname=None):
         ret_val.append(Result(
             priority,
             value=res,
-            name=gname if gname else l,
+            name=gname if gname else name,
             msgs=msgs
         ))
+    elif hasattr(other, '__iter__'):
+        # redundant, we could easily do this with a hasattr
+        # check instead
+        res = std_check_in(ds, name, other)
+        if res == 0:
+            msgs.append("%s not present" % name)
+        elif res == 1:
+            msgs.append("%s present, but not in expected value list (%s)" % (name, other))
+
+        ret_val.append(
+            Result(
+                priority,
+                (res, 2),
+                gname if gname else name, # groups Globals if supplied
+                msgs
+            )
+        )
+    # if we have an XPath expression, call it on the document
+    elif type(other) is etree.XPath:
+        # TODO: store tree instead of creating it each time?
+        res = xpath_check(ds._root, other)
+        if not res:
+            msgs = ["XPath for {} not found".format(name)]
+        ret_val.append(
+            Result(
+                priority,
+                res,
+                gname if gname else name,
+                msgs
+            )
+        )
+    # if the attribute is a function, call it
+    # right now only supports single attribute
+    # important note: current magic approach uses all functions
+    # starting with "check".  Avoid naming check functions
+    # starting with check if you want to pass them in with
+    # a tuple to avoid them being checked more than once
+    elif hasattr(other, '__call__'):
+        # check that the attribute is actually present.
+        # This reduces boilerplate in functions by not needing
+        # to check whether the attribute is present every time
+        # and instead focuses on the core functionality of the
+        # test
+
+        res = other(ds) # call the method on the dataset
+        if not res:
+            msgs = ["%s not present" % name]
+            ret_val.append(
+                Result(
+                    priority,
+                    res,
+                    gname if gname else name,
+                    msgs
+                )
+            )
+        else:
+            ret_val.append(res(priority))
+    # unsupported second type in second
+    else:
+        raise TypeError("Second arg in tuple has unsupported type: {}".format(type(other)))
+
 
     return ret_val
 
@@ -342,14 +342,14 @@ def check_has(priority=BaseCheck.HIGH, gname=None):
 
     def _inner(func):
         def _dec(s, ds):
-            list_vars = func(s, ds)
+            attr_process = kvp_convert(func(s, ds))
 
             ret_val = []
             # could potentially run tests in parallel if we eliminated side
             # effects on `ret_val`
-            for l in list_vars:
+            for kvp in attr_process.items():
                 # function mutates ret_val
-                attr_check(l, ds, priority, ret_val, gname)
+                attr_check(kvp, ds, priority, ret_val, gname)
             return ret_val
 
         return wraps(func)(_dec)
diff --git a/compliance_checker/util.py b/compliance_checker/util.py
index 671f0d8..87314b1 100644
--- a/compliance_checker/util.py
+++ b/compliance_checker/util.py
@@ -3,6 +3,7 @@ General purpose utility functions to aid in compliance checking tasks
 """
 import isodate
 import pendulum
+from collections import OrderedDict
 
 
 def isstring(obj):
@@ -33,3 +34,19 @@ def dateparse(date_str):
 
     return pendulum.parse(date_str)
 
+
+def kvp_convert(input_coll):
+    '''
+    Converts a list of string attributes and/or tuples into an OrderedDict.
+    If passed in an OrderedDict, function is idempotent.
+    Key/value pairs map to `first_tuple_element` -> `second_tuple_element` if
+    a tuple, or `scalar_value` -> None if not a tuple.
+
+    :param input_coll: An iterable with string and/or 2-tuple elements
+    :returns: collections.OrderedDict
+    '''
+    if isinstance(input_coll, OrderedDict):
+        return input_coll
+    else:
+        return OrderedDict((l, None) if not isinstance(l, tuple) else
+                           (l[0], l[1]) for l in input_coll)


Test patch:
diff --git a/compliance_checker/tests/test_base.py b/compliance_checker/tests/test_base.py
index 87863bd..cf7e260 100644
--- a/compliance_checker/tests/test_base.py
+++ b/compliance_checker/tests/test_base.py
@@ -25,7 +25,7 @@ class TestBase(TestCase):
         # attribute not present, should fail
         priority = base.BaseCheck.MEDIUM
         rv1, rv2, rv3, rv4 = [], [], [], []
-        attr = 'test'
+        attr = ('test', None)
         base.attr_check(attr, self.ds, priority, rv1)
         assert rv1[0] == base.Result(priority, False, "test",
                                      ['test not present'])
@@ -99,4 +99,4 @@ class TestGenericFile(TestCase):
 
     def test_create_GenericFile_failure(self):
         gf = base.GenericFile("will not match")
-        self.assertNotEqual(gf.filepath(), "do NOT MATCH") 
+        self.assertNotEqual(gf.filepath(), "do NOT MATCH")