Repository: ioos/compliance-checker
Pull request #616 (2018-12-03 19:34:55)

Eliminates tabular standard output
Address #578


Patch:
diff --git a/README.md b/README.md
index aa50266..d0d2d5a 100644
--- a/README.md
+++ b/README.md
@@ -140,15 +140,7 @@ optional arguments:
                         Normal, or Lenient. Defaults to Normal.
   --verbose, -v         Increase output. May be specified up to three times.
   --skip-checks SKIP_CHECKS, -s SKIP_CHECKS
-                        Specifies tests to skip. Can take the form of either
-                        `<check_name>` or `<check_name>:<skip_level>`.  The
-                        first form skips any checks matching the name.  In the
-                        second form <skip_level> may be specified as "A", "M",
-                        or "L".  "A" skips all checks and is equivalent to
-                        calling the first form. "M" will only show high
-                        priority output from the given check and will skip
-                        medium and low.  "L" will show both high and medium
-                        priority issues, while skipping low priority issues.
+                        Specifies tests to skip
   -f {text,html,json,json_new}, --format {text,html,json,json_new}
                         Output format. The difference between the 'json' and
                         the 'json_new' formats is that the 'json' format has
diff --git a/cchecker.py b/cchecker.py
index f6a393a..1b49f9e 100755
--- a/cchecker.py
+++ b/cchecker.py
@@ -6,7 +6,6 @@ import sys
 from compliance_checker.runner import ComplianceChecker, CheckSuite
 from compliance_checker.cf.util import download_cf_standard_name_table
 from compliance_checker import __version__
-from textwrap import dedent
 
 
 def main():
@@ -34,20 +33,7 @@ def main():
                         default=0)
 
     parser.add_argument('--skip-checks', '-s',
-                        help=dedent("""
-                                    Specifies tests to skip. Can take the form
-                                    of either `<check_name>` or
-                                    `<check_name>:<skip_level>`.  The first
-                                    form skips any checks matching the name.
-                                    In the second form <skip_level> may be
-                                    specified as "A", "M", or "L".  "A" skips
-                                    all checks and is equivalent to calling
-                                    the first form. "M" will only show high
-                                    priority output from the given check and
-                                    will skip medium and low.  "L" will show
-                                    both high and medium priority issues, while
-                                    skipping low priority issues.
-                                    """),
+                        help="Specifies tests to skip",
                         action='append')
 
     parser.add_argument('-f', '--format', default=[], action='append',
diff --git a/compliance_checker/runner.py b/compliance_checker/runner.py
index 6e78b72..e864c39 100644
--- a/compliance_checker/runner.py
+++ b/compliance_checker/runner.py
@@ -30,7 +30,6 @@ class ComplianceChecker(object):
     Ties together the entire compliance checker framework, is used from
     the command line or can be used via import.
     """
-    # Consider using __init__ instead of so many classmethods
     @classmethod
     def run_checker(cls, ds_loc, checker_names, verbose, criteria,
                     skip_checks=None, output_filename='-',
@@ -65,7 +64,8 @@ class ComplianceChecker(object):
         for loc in locs:
             ds = cs.load_dataset(loc)
 
-            score_groups = cs.run(ds, skip_checks, *checker_names)
+            score_groups = cs.run(ds, [] if skip_checks is None else skip_checks,
+                                *checker_names)
             # TODO: consider wrapping in a proper context manager instead
             if hasattr(ds, 'close'):
                 ds.close()
diff --git a/compliance_checker/suite.py b/compliance_checker/suite.py
index bf54fa3..27b7b03 100644
--- a/compliance_checker/suite.py
+++ b/compliance_checker/suite.py
@@ -17,23 +17,17 @@ from compliance_checker.base import fix_return_value, Result, GenericFile
 from owslib.sos import SensorObservationService
 from owslib.swe.sensor.sml import SensorML
 from compliance_checker.protocols import opendap, netcdf, cdl
-from compliance_checker.base import BaseCheck
 from compliance_checker import MemoizedDataset
-from collections import defaultdict
-import warnings
 try:
     from urlparse import urlparse
 except ImportError:
     from urllib.parse import urlparse
 from datetime import datetime
 import requests
-import textwrap
 import codecs
 from pkg_resources import working_set
-import tabulate
 
 
-tabulate.PRESERVE_WHITESPACE = True
 # Ensure output is encoded as Unicode when checker output is redirected or piped
 if sys.stdout.encoding is None:
     sys.stdout = codecs.getwriter('utf8')(sys.stdout)
@@ -43,10 +37,9 @@ if sys.stderr.encoding is None:
 class CheckSuite(object):
 
     checkers = {}       # Base dict of checker names to BaseCheck derived types, override this in your CheckSuite implementation
-    templates_root = 'compliance_checker'  # modify to load alternative Jinja2 templates
 
     def __init__(self):
-        self.col_width = 60
+        self.col_width = 40
 
     @classmethod
     def _get_generator_plugins(cls):
@@ -126,36 +119,16 @@ class CheckSuite(object):
         """
         meths = inspect.getmembers(checkclass, inspect.ismethod)
         # return all check methods not among the skipped checks
-        returned_checks = []
-        for fn_name, fn_obj in meths:
-            if (fn_name.startswith("check_") and
-                skip_checks[fn_name] != BaseCheck.HIGH):
-                returned_checks.append((fn_obj, skip_checks[fn_name]))
+        return [x[1] for x in meths if x[0].startswith("check_") and
+                x[0] not in skip_checks]
 
-        return returned_checks
-
-    def _run_check(self, check_method, ds, max_level):
-        """
-        Runs a check and appends a result to the
-        """
+    def _run_check(self, check_method, ds):
         val = check_method(ds)
 
         if isinstance(val, list):
-            check_val = []
-            for v in val:
-                res = fix_return_value(v, check_method.__func__.__name__,
-                                       check_method, check_method.__self__)
-                if max_level is None or res.weight > max_level:
-                    check_val.append(res)
-
-            return check_val
-        else:
-            check_val = fix_return_value(val, check_method.__func__.__name__,
-                                         check_method, check_method.__self__)
-            if max_level is None or check_val.weight > max_level:
-                return [check_val]
-            else:
-                return []
+            return [fix_return_value(v, check_method.__func__.__name__, check_method, check_method.__self__) for v in val]
+
+        return [fix_return_value(val, check_method.__func__.__name__, check_method, check_method.__self__)]
 
     def _get_check_versioned_name(self, check_name):
         """
@@ -208,39 +181,6 @@ class CheckSuite(object):
 
         return valid
 
-
-    @classmethod
-    def _process_skip_checks(cls, skip_checks):
-        """
-        Processes an iterable of skip_checks with strings and returns a dict
-        with <check_name>: <max_skip_level> pairs
-        """
-
-        check_dict = defaultdict(lambda: None)
-        # A is for "all", "M" is for medium, "L" is for low
-        check_lookup = {'A': BaseCheck.HIGH,
-                        'M': BaseCheck.MEDIUM,
-                        'L': BaseCheck.LOW}
-
-        for skip_check_spec in skip_checks:
-            split_check_spec = skip_check_spec.split(':')
-            check_name = split_check_spec[0]
-            if len(split_check_spec) < 2:
-               check_max_level = BaseCheck.HIGH
-            else:
-                try:
-                    check_max_level = check_lookup[split_check_spec[1]]
-                except KeyError:
-                    warnings.warn("Skip specifier '{}' on check '{}' not found,"
-                                  " defaulting to skip entire check".format(split_check_spec[1], check_name))
-                    check_max_level = BaseCheck.HIGH
-
-            check_dict[check_name] = check_max_level
-
-        return check_dict
-
-
-
     def run(self, ds, skip_checks, *checker_names):
         """
         Runs this CheckSuite on the dataset with all the passed Checker instances.
@@ -251,11 +191,6 @@ class CheckSuite(object):
         ret_val = {}
         checkers = self._get_valid_checkers(ds, checker_names)
 
-        if skip_checks is not None:
-            skip_check_dict = CheckSuite._process_skip_checks(skip_checks)
-        else:
-            skip_check_dict = defaultdict(lambda: None)
-
         if len(checkers) == 0:
             print("No valid checkers found for tests '{}'".format(",".join(checker_names)))
 
@@ -264,13 +199,13 @@ class CheckSuite(object):
             checker = checker_class()
             checker.setup(ds)
 
-            checks = self._get_checks(checker, skip_check_dict)
+            checks = self._get_checks(checker, skip_checks)
             vals = []
             errs = {}   # check method name -> (exc, traceback)
 
-            for c, max_level in checks:
+            for c in checks:
                 try:
-                    vals.extend(self._run_check(c, ds, max_level))
+                    vals.extend(self._run_check(c, ds))
                 except Exception as e:
                     errs[c.__func__.__name__] = (e, sys.exc_info()[2])
 
@@ -414,7 +349,7 @@ class CheckSuite(object):
         @param limit           Integer value for limiting output
         '''
         from jinja2 import Environment, PackageLoader
-        self.j2 = Environment(loader=PackageLoader(self.templates_root, 'data/templates'))
+        self.j2 = Environment(loader=PackageLoader('compliance_checker', 'data/templates'))
         template = self.j2.get_template('ccheck.html.j2')
 
         template_vars = self.build_structure(check_name, groups, source_name, limit)
@@ -461,7 +396,7 @@ class CheckSuite(object):
         # Let's add the version number to the check name if it's missing
         check_name = self._get_check_versioned_name(check_name)
         check_url = self._get_check_url(check_name)
-        width = 2*self.col_width
+        width = 2 * self.col_width
         print('\n')
         print("-" * width)
         print('{:^{width}}'.format("IOOS Compliance Checker Report", width=width))
@@ -495,14 +430,11 @@ class CheckSuite(object):
         result = {key: [v for v in valuesiter if v.value[0] != v.value[1]]
                     for key, valuesiter in itertools.groupby(groups_sorted,
                                                              key=sort_fn)}
-        wrapper = textwrap.TextWrapper(initial_indent='',
-                                       width=max(int(self.col_width / 2**indent), self.col_width / 2))
-
         priorities = self.checkers[check]._cc_display_headers
         def process_table(res, check):
-            issue = wrapper.fill("{}:".format(res.name))
+            issue = res.name
             if not res.children:
-                reason = wrapper.fill('\n'.join(res.msgs))
+                reasons = res.msgs
             else:
                 child_reasons = self.reasoning_routine(res.children,
                                                        indent + 1,
@@ -510,11 +442,12 @@ class CheckSuite(object):
                                                         _top_level=False)
                 # there shouldn't be messages if there are children
                 # is this a valid assumption?
-                reason = "\n{}".format(child_reasons)
+                reasons = child_reasons
 
-            return issue, reason
+            return issue, reasons
 
         # iterate up to the min priority requested
+        proc_strs = ""
         for level in range(3, priority_flag - 1, -1):
             level_name = priorities.get(level, level)
             # print headers
@@ -536,13 +469,17 @@ class CheckSuite(object):
 
                 data_issues = [process_table(res, check) for res in result[level]]
 
-                if _top_level:
-                    proc_str = tabulate.tabulate(data_issues, ('Name', 'Reasoning'),
-                                        'plain')
+                has_printed = False
+                for issue, reasons in data_issues:
+                    # if this isn't the first printed issue, add a newline
+                    # separating this and the previous level
+                    if has_printed:
+                        print("")
+                    reason_str = "\n".join('* {}'.format(r) for r in reasons)
+                    proc_str = "{}\n{}".format(issue, reason_str)
                     print(proc_str)
-                else:
-                    proc_str = tabulate.tabulate(data_issues, tablefmt='plain')
-                proc_strs.append(proc_str)
+                    proc_strs.append(proc_str)
+                    has_printed = True
         return "\n".join(proc_strs)
 
 
diff --git a/requirements.txt b/requirements.txt
index ce9f0df..e6fcade 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -9,7 +9,6 @@ pygeoif>=0.6
 netCDF4>=1.4.0
 regex==2017.07.28
 pendulum>=1.2.4
-tabulate==0.8.2
 # functools.lru_cache first appears in Python 3.2, use this for other
 # versions
 functools32==3.2.3-2; python_version < '3.2' #conda: functools32  (only python=2)
diff --git a/setup.py b/setup.py
index 21e8a1e..6aa9855 100644
--- a/setup.py
+++ b/setup.py
@@ -35,8 +35,6 @@ setup(
         'Intended Audience :: Science/Research',
         'License :: OSI Approved :: Apache Software License',
         'Operating System :: POSIX :: Linux',
-        'Operating System :: MacOS :: MacOS X',
-        'Operating System :: Microsoft :: Windows',
         'Programming Language :: Python',
         'Topic :: Scientific/Engineering',
     ],


Test patch:
diff --git a/compliance_checker/tests/test_suite.py b/compliance_checker/tests/test_suite.py
index f42578a..2c1816c 100644
--- a/compliance_checker/tests/test_suite.py
+++ b/compliance_checker/tests/test_suite.py
@@ -1,4 +1,3 @@
-# coding=utf-8
 from pkg_resources import resource_filename
 from compliance_checker.suite import CheckSuite
 from compliance_checker.base import Result, BaseCheck, GenericFile
@@ -7,13 +6,12 @@ import unittest
 import os
 
 static_files = {
-    '2dim'         : resource_filename('compliance_checker', 'tests/data/2dim-grid.nc'),
-    'bad_region'   : resource_filename('compliance_checker', 'tests/data/bad_region.nc'),
-    'bad_data_type': resource_filename('compliance_checker', 'tests/data/bad_data_type.nc'),
-    'test_cdl'     : resource_filename('compliance_checker', 'tests/data/test_cdl.cdl'),
-    'test_cdl_nc'  : resource_filename('compliance_checker', 'tests/data/test_cdl_nc_file.nc'),
-    'empty'        : resource_filename('compliance_checker', 'tests/data/non-comp/empty.file'),
-    'ru07'         : resource_filename('compliance_checker', 'tests/data/ru07-20130824T170228_rt0.nc')
+    '2dim'                       : resource_filename('compliance_checker', 'tests/data/2dim-grid.nc'),
+    'bad_region'                 : resource_filename('compliance_checker', 'tests/data/bad_region.nc'),
+    'bad_data_type'              : resource_filename('compliance_checker', 'tests/data/bad_data_type.nc'),
+    'test_cdl'                   : resource_filename('compliance_checker', 'tests/data/test_cdl.cdl'),
+    'test_cdl_nc'                : resource_filename('compliance_checker', 'tests/data/test_cdl_nc_file.nc'),
+    'empty'                    : resource_filename('compliance_checker', 'tests/data/non-comp/empty.file'),
 }
 
 
@@ -67,33 +65,6 @@ class TestSuite(unittest.TestCase):
         assert all(sg.name not in {'Conventions', 'title', 'keywords',
                                    'summary'} for sg in score_groups['acdd'][0])
 
-    def test_skip_check_level(self):
-        """Checks level limited skip checks"""
-        ds = self.cs.load_dataset(static_files['ru07'])
-        score_groups = self.cs.run(ds, ['check_flags:A',
-                                        'check_convention_possibly_var_attrs:M',
-                                        'check_standard_name:L'], 'cf')
-
-        name_set = {sg.name for sg in score_groups['cf'][0]}
-        # flattened set of messages
-        msg_set = {msg for sg in score_groups['cf'][0] for msg in sg.msgs}
-
-        expected_excluded_names = {u'§3.5 flag_meanings for lat',
-                                   u'§3.5 flag_meanings for lon',
-                                   u'§3.5 lat is a valid flags variable',
-                                   u'§3.5 lat is a valid flags variable',
-                                   u'§3.5 lon is a valid flags variable'}
-
-        self.assertTrue(len(expected_excluded_names & name_set) == 0)
-
-        # should skip references
-        ref_msg = u'references global attribute should be a non-empty string'
-        self.assertTrue(ref_msg not in msg_set)
-        # check_standard_name is high priority, but we requested only low,
-        # so the standard_name check should still exist
-        standard_name_hdr = u'§3.3 Variable salinity has valid standard_name attribute'
-        self.assertTrue(standard_name_hdr in name_set)
-
     def test_group_func(self):
         # This is checking for issue #183, where group_func results in
         # IndexError: list index out of range