Merge pull request #143 from rillian/warnings

Warning cleanup

.travis.yml

@@ -7,11 +7,12 @@ python:
 # command to install dependencies
 install:
     - pip install -r requirements.txt
-    - pip install coveralls
+    - pip install coveralls pycodestyle
 
 # command to run tests
 script:
     - coverage run setup.py test
+    - pycodestyle --ignore E501 *.py HookTest
 after_success:
     - coverage combine
     - coveralls

HookTest/build.py

@@ -80,10 +80,7 @@ def remove_failing(self, files, passing):
                     continue
         # Covers the case where the source and destination directories are different, so files are copied instead of removed
         else:
-            try:
-                shutil.rmtree('{}data'.format(self.dest))
-            except:
-                pass
+            shutil.rmtree('{}data'.format(self.dest), ignore_errors=True)
             for file in files:
                 if file.replace(self.path, '') in passing:
                     try:
@@ -175,4 +172,4 @@ def cmd(**kwargs):
                                  txt=kwargs['txt'], cites=kwargs['cites'], workers=int(kwargs['workers'])).run()
         return status, message
     else:
-        return False, 'You cannot run build on the base class'
+        return False, 'You cannot run build on the base class'

HookTest/capitains_units/cts.py

@@ -173,15 +173,15 @@ def check_urns(self):
                 onlyOneWork = True
                 allMembers = True
                 worksUrns = [
-                        urn
-                        for urn in self.xml.xpath("//ti:work/@urn", namespaces=TESTUnit.NS)
-                        if urn and len(MyCapytain.common.reference.URN(urn)) == 4
-                    ]
+                    urn
+                    for urn in self.xml.xpath("//ti:work/@urn", namespaces=TESTUnit.NS)
+                    if urn and len(MyCapytain.common.reference.URN(urn)) == 4
+                ]
                 groupUrns = [
-                        urn
-                        for urn in self.xml.xpath("//ti:work/@groupUrn", namespaces=TESTUnit.NS)
-                        if urn and len(MyCapytain.common.reference.URN(urn)) == 3
-                    ]
+                    urn
+                    for urn in self.xml.xpath("//ti:work/@groupUrn", namespaces=TESTUnit.NS)
+                    if urn and len(MyCapytain.common.reference.URN(urn)) == 3
+                ]
                 self.urn = None
                 urn = None
                 if len(worksUrns) == 1:
@@ -230,10 +230,10 @@ def check_urns(self):
 
                 self.log("Edition, translation, and commentary urns : " + " ".join(self.urns))
 
-                status = allMembers and\
-                         matches and onlyOneWork and self.urn and \
-                            len(groupUrns) == 1 and \
-                            (len(texts)*2+1) == len(self.urns + worksUrns)
+                status = allMembers and \
+                    matches and onlyOneWork and self.urn and \
+                    len(groupUrns) == 1 and \
+                    (len(texts) * 2 + 1) == len(self.urns + worksUrns)
 
         yield status
 
@@ -396,7 +396,7 @@ def run_rng(self, rng_path):
         :param rng_path: Path to the RelaxNG file to run against the XML to test
         """
         test = subprocess.Popen(
-            ["java", "-Duser.country=US",  "-Duser.language=en", "-jar", TESTUnit.JING, rng_path, self.path],
+            ["java", "-Duser.country=US", "-Duser.language=en", "-jar", TESTUnit.JING, rng_path, self.path],
             stdout=subprocess.PIPE,
             stderr=subprocess.PIPE,
             shell=False
@@ -412,7 +412,7 @@ def run_rng(self, rng_path):
             yield False
             pass
         finally:
-            if not timer.isAlive():
+            if not timer.is_alive():
                 self.log("Timeout on RelaxNG")
                 yield False
                 timer.cancel()
@@ -459,8 +459,8 @@ def get_remote_rng(self, url):
         # We have a name for the rng file but also for the in-download marker
         # Note : we might want to add a os.makedirs somewhere with exists=True
         makedirs(".rngs", exist_ok=True)
-        stable_local = os.path.join(".rngs", sha+".rng")
-        stable_local_downloading = os.path.join(".rngs", sha+".rng-indownload")
+        stable_local = os.path.join(".rngs", sha + ".rng")
+        stable_local_downloading = os.path.join(".rngs", sha + ".rng-indownload")
 
         # check if the stable_local rng already exists
         # if it does, immediately run the rng test and move to the next rng in the file
@@ -521,7 +521,7 @@ def passages(self):
                     with warnings.catch_warnings(record=True) as warning_record:
                         # Cause all warnings to always be triggered.
                         warnings.simplefilter("always")
-                        passages = self.Text.getValidReff(level=i+1, _debug=True)
+                        passages = self.Text.getValidReff(level=i + 1, _debug=True)
                         ids = [ref.split(".", i)[-1] for ref in passages]
                         space_in_passage = TESTUnit.FORBIDDEN_CHAR.search("".join(ids))
                         len_passage = len(passages)
@@ -543,7 +543,7 @@ def passages(self):
                         yield status
                 except Exception as E:
                     self.error(E)
-                    self.log("Error when searching passages at level {0}".format(i+1))
+                    self.log("Error when searching passages at level {0}".format(i + 1))
                     yield False
                     break
         else:
@@ -615,8 +615,12 @@ def has_urn(self):
         """
         if self.xml is not None:
             if self.guidelines == "2.tei":
-                urns = self.xml.xpath("//tei:text/tei:body[starts-with(@n, 'urn:cts:')]", namespaces=TESTUnit.NS) + \
-                        self.xml.xpath("//tei:text[starts-with(@xml:base, 'urn:cts:')]", namespaces=TESTUnit.NS)
+                urns = self.xml.xpath(
+                    "//tei:text/tei:body[starts-with(@n, 'urn:cts:')]",
+                    namespaces=TESTUnit.NS)
+                urns += self.xml.xpath(
+                    "//tei:text[starts-with(@xml:base, 'urn:cts:')]",
+                    namespaces=TESTUnit.NS)
             else:
                 urns = self.xml.xpath(
                     "//tei:body/tei:div[@type='edition' and starts-with(@n, 'urn:cts:')]",
@@ -694,11 +698,11 @@ def language(self):
             )
         elif self.guidelines == "2.tei":
             urns_holding_node = self.xml.xpath("//tei:text/tei:body[starts-with(@n, 'urn:cts:')]", namespaces=TESTUnit.NS) + \
-                    self.xml.xpath("//tei:text[starts-with(@xml:base, 'urn:cts:')]", namespaces=TESTUnit.NS)
+                self.xml.xpath("//tei:text[starts-with(@xml:base, 'urn:cts:')]", namespaces=TESTUnit.NS)
 
         try:
             self.lang = urns_holding_node[0].get('{http://www.w3.org/XML/1998/namespace}lang')
-        except:
+        except IndexError:
             self.lang = ''
         if self.lang == '' or self.lang is None:
             self.lang = 'UNK'
@@ -740,8 +744,8 @@ def test(self, scheme, guidelines, rng=None, inventory=None):
             status = False not in [status for status in getattr(self, test)()]
             self.test_status[test] = status
             yield (CTSText_TestUnit.readable[test], status, self.logs)
-            if test in self.breaks and status == False:
-                for t in tests[i+1:]:
+            if test in self.breaks and not status:
+                for t in tests[i + 1:]:
                     self.test_status[t] = False
                     yield (CTSText_TestUnit.readable[t], False, [])
                 break

HookTest/test.py

@@ -20,11 +20,11 @@
 
 import HookTest.capitains_units.cts
 import HookTest.units
-from colors import white, magenta, black
+from colors import white, magenta
 from operator import attrgetter
 
 
-pr_finder = re.compile("pull\/([0-9]+)\/head")
+pr_finder = re.compile("pull/([0-9]+)/head")
 
 
 class DefaultFinder(object):
@@ -82,9 +82,9 @@ def find(self, directory):
         cts = glob.glob(os.path.join(directory, "data/{textgroup}/__cts__.xml".format(
             textgroup=textgroup
         ))) + \
-              glob.glob(os.path.join(directory, "data/{textgroup}/{work}/__cts__.xml".format(
-                  textgroup=textgroup, work=work
-              )))
+            glob.glob(os.path.join(directory, "data/{textgroup}/{work}/__cts__.xml".format(
+                textgroup=textgroup, work=work
+            )))
         files = glob.glob(os.path.join(directory, "data/{textgroup}/{work}/{version}.xml".format(
             textgroup=textgroup, work=work, version=version
         )))
@@ -457,7 +457,7 @@ def start(self):
             self.scheme = "auto_rng"
         if self.console:
             print(">>> Starting tests !", flush=True)
-            print(">>> Files to test : "+str(self.count_files), flush=True)
+            print(">>> Files to test : " + str(self.count_files), flush=True)
         elif self.ping:
             self.send({
                 "logs": [
@@ -534,7 +534,7 @@ def end(self):
                         try:
                             show.remove("Duplicate passages")
                             show.remove("Forbidden characters")
-                        except:
+                        except ValueError:
                             pass
                     if unit.coverage != 100.0:
                         num_failed += 1
@@ -548,36 +548,41 @@ def end(self):
                         failed_tests = '\n'.join([x for x in unit.units if unit.units[x] is False and x in show])
 
                     if unit.additional['duplicates']:
-                        duplicate_nodes += '\t{name}\t{nodes}\n'.format(name=magenta(os.path.basename(unit.name)),
-                                                                      nodes=', '.join(unit.additional['duplicates']))
+                        duplicate_nodes += '\t{name}\t{nodes}\n'.format(
+                            name=magenta(os.path.basename(unit.name)),
+                            nodes=', '.join(unit.additional['duplicates']))
                     if unit.additional['forbiddens']:
-                        forbidden_chars += '\t{name}\t{nodes}\n'.format(name=magenta(os.path.basename(unit.name)),
-                                                                      nodes=', '.join(unit.additional['forbiddens']))
+                        forbidden_chars += '\t{name}\t{nodes}\n'.format(
+                            name=magenta(os.path.basename(unit.name)),
+                            nodes=', '.join(unit.additional['forbiddens']))
                     if unit.additional["dtd_errors"] and self.verbose >= 6:
-                        dtd_errors += '\t{name}\t{nodes}\n'.format(name=magenta(os.path.basename(unit.name)),
-                                                                   nodes=', '.join(unit.additional["dtd_errors"]))
+                        dtd_errors += '\t{name}\t{nodes}\n'.format(
+                            name=magenta(os.path.basename(unit.name)),
+                            nodes=', '.join(unit.additional["dtd_errors"]))
 
                     if unit.additional["capitains_errors"]:
-                        capitains_errors += '\t{name}\t{nodes}\n'.format(name=magenta(os.path.basename(unit.name)),
-                                                                   nodes=', '.join(unit.additional["capitains_errors"]))
+                        capitains_errors += '\t{name}\t{nodes}\n'.format(
+                            name=magenta(os.path.basename(unit.name)),
+                            nodes=', '.join(unit.additional["capitains_errors"]))
 
                     if unit.additional["empties"]:
-                        empty_refs += '\t{name}\t{nodes}\n'.format(name=magenta(os.path.basename(unit.name)),
-                                                                   nodes=', '.join(unit.additional["empties"]))
+                        empty_refs += '\t{name}\t{nodes}\n'.format(
+                            name=magenta(os.path.basename(unit.name)),
+                            nodes=', '.join(unit.additional["empties"]))
 
                     if self.verbose >= 7 or unit.status is False:
                         if self.countwords:
                             row = [
                                 "{}".format(text_color(os.path.basename(unit.name))),
-                                 "{:,}".format(unit.additional['words']),
-                                 ';'.join([str(x[1]) for x in unit.additional['citations']]),
-                                 failed_tests
+                                "{:,}".format(unit.additional['words']),
+                                ';'.join([str(x[1]) for x in unit.additional['citations']]),
+                                failed_tests
                             ]
                         else:
                             row = [
                                 "{}".format(text_color(os.path.basename(unit.name))),
-                                 ';'.join([str(x[1]) for x in unit.additional['citations']]),
-                                 failed_tests
+                                ';'.join([str(x[1]) for x in unit.additional['citations']]),
+                                failed_tests
                             ]
                         display_table.add_row(row)
 
@@ -605,7 +610,7 @@ def end(self):
 
             if capitains_errors:
                 capitains_errors = magenta('CapiTainS parsing errors found:\n') + capitains_errors + '\n'
-                
+
             print("{caps}{dupes}{forbs}{dtds}{empts}>>> End of the test !\n".format(caps=capitains_errors,
                                                                                     dupes=duplicate_nodes,
                                                                                     forbs=forbidden_chars,
@@ -745,7 +750,7 @@ def cover(self, name, test, testtype=None, logs=None, additional=None):
                 directory=self.directory,
                 name=name,
                 units=test,
-                coverage=len([v for v in results if v is True])/len(results)*100,
+                coverage=len([v for v in results if v is True]) / len(results) * 100,
                 status=False not in results,
                 logs=logs,
                 additional=additional,

HookTest/units.py

@@ -14,10 +14,10 @@ class TESTUnit(object):
     EPIDOC = pkg_resources.resource_filename("HookTest", "resources/epidoc.rng")
     TEI_ALL = pkg_resources.resource_filename("HookTest", "resources/tei.rng")
     JING = pkg_resources.resource_filename("jingtrang", "jing.jar")
-    RNG_ERROR = re.compile("([0-9]+):([0-9]+):(.*);")
-    RNG_FAILURE = re.compile("([0-9]+):([0-9]+):(\s*fatal.*)")
-    SPACE_REPLACER = re.compile("(\s{2,})")
-    FORBIDDEN_CHAR = re.compile("[^\w\d]")
+    RNG_ERROR = re.compile(r"([0-9]+):([0-9]+):(.*);")
+    RNG_FAILURE = re.compile(r"([0-9]+):([0-9]+):(\s*fatal.*)")
+    SPACE_REPLACER = re.compile(r"(\s{2,})")
+    FORBIDDEN_CHAR = re.compile(r"[^\w\d]")
     NS = {"tei": "http://www.tei-c.org/ns/1.0", "ti": "http://chs.harvard.edu/xmlns/cts"}
     PARSER = etree.XMLParser(no_network=True, resolve_entities=False)
 
@@ -33,11 +33,11 @@ def __init__(self, path):
     @property
     def logs(self):
         return self.__logs
-    
+
     def log(self, message):
         if isinstance(message, str) and not message.isspace() and len(message) > 0:
             self.__logs.append(">>>>>> " + TESTUnit.SPACE_REPLACER.sub(" ", message.lstrip()))
-    
+
     def error(self, error):
         if isinstance(error, Exception):
             self.log(str(type(error)) + " : " + str(error))
@@ -100,5 +100,3 @@ def rng_logs(logs):
 
         for key, value in filtered_logs.items():
             yield "{0} [In {1}]".format(key, "; ".join(value))
-
-

evaluate.py

@@ -13,4 +13,4 @@
     "../canonical-lat-test",
     "-w", "8", "--countword", "--scheme", "epidoc",
     "--allowfailure", "--console", "table", "--verbose", "10"
-])))
+])))

setup.py

@@ -1,3 +1,5 @@
+# -*- coding: utf-8 -*-
+
 from setuptools import setup, find_packages
 from codecs import open  # To use a consistent encoding
 from os import path
@@ -14,8 +16,8 @@
     now_we_have_dev = False
     for line in f:
         if line.startswith("#"):
-             if "test" in line:
-                   now_we_have_dev = True
+            if "test" in line:
+                now_we_have_dev = True
         elif now_we_have_dev:
             tests_require.append(line.strip())
         else:

tests/test_cts/mocks/xmllang_data.py

@@ -38,7 +38,8 @@
     ("epidoc", "<body>", "<body n='{urn}'>", False, "Epidoc fails with urn without xml lang on @n/body"),
 
 ]
-# Epidocs Tests
+
+# Epidocs Tests
 XMLLANG_DOCUMENTS += [
     (scheme, source, replacement.replace("{epidoc}", type_epidoc), boolean, message.replace("{epidoc}", type_epidoc))
     for scheme, source, replacement, boolean, message in
@@ -77,4 +78,4 @@
         msg + " ("+replacement.format(urn=URN, lang=LANG)+")"
     )
     for scheme, source, replacement, boolean, msg in XMLLANG_DOCUMENTS
-]
+]

tests/test_run.py

@@ -35,7 +35,7 @@ def assertLogResult(self, logs, left_column, right_column, message):
         :param right_column: Right Column content
         :param message: Message to show for failure
         """
-        self.assertRegex(logs, "\|\s+"+left_column+"\s+\|\s+" + right_column + "\s+\|", message)
+        self.assertRegex(logs, r"\|\s+" + left_column + r"\s+\|\s+" + right_column + r"\s+\|", message)
 
     def parse_subset(self, logs, file):
         """
@@ -45,11 +45,11 @@ def parse_subset(self, logs, file):
         :return:
         """
         regex = re.compile(
-            "(?:("+re.escape(white(file))+")|(" + re.escape(magenta(file)) + "))"  # Starts with the file name
-            "\s+\|\s+([0-9;]+)\s+\|\s+"  # Nodes Count
-            "([a-zA-Z0-9 \n\|\:\;]+)\+---"  # Colonnes
+            r"(?:(" + re.escape(white(file)) + ")|(" + re.escape(magenta(file)) + "))"  # Starts with the file name
+            r"\s+\|\s+([0-9;]+)\s+\|\s+"     # Nodes Count
+            r"([a-zA-Z0-9 \n\|\:\;]+)\+---"  # Colonnes
         )
-        regex_tests = re.compile("(?:(?:\|\s+)+)?((?:[A-Za-z0-9:]+\s)+)")
+        regex_tests = re.compile(r"(?:(?:\|\s+)+)?((?:[A-Za-z0-9:]+\s)+)")
         for _, match, nodes, text in regex.findall(logs):
             tests = [l.strip() for l in regex_tests.findall(text)]
             return nodes, tests
@@ -238,15 +238,15 @@ def test_run_local_console(self):
             "3 texts should be described in logs"
         )
         self.assertRegex(
-            logs, "hafez\.divan\.perseus-far1\.xml",
+            logs, r"hafez\.divan\.perseus-far1\.xml",
             "Far1 file should be named"
         )
         self.assertRegex(
-            logs, "hafez\.divan\.perseus-eng1\.xml",
+            logs, r"hafez\.divan\.perseus-eng1\.xml",
             "eng1 file should be named"
         )
         self.assertRegex(
-            logs, "hafez\.divan\.perseus-ger1\.xml",
+            logs, r"hafez\.divan\.perseus-ger1\.xml",
             "ger1 file should be named"
         )
         self.assertEqual(status, "failed", "Test should fail")