lint the code

.flake8

@@ -1,4 +1,7 @@
 [flake8]
-ignore = E203, E266, E501, W503, E211, E731
-max-line-length = 88
-select = B,C,E,F,W,T4,B9
+max-line-length = 100
+select = B,C,E,F,W,T4
+exclude = cwltool/schemas
+extend-ignore = E203,E501,E704,B905,W503
+# when Python 3.10 is the minimum version, re-enable check B905 for zip + strict
+extend-select = B9

dev-requirements.txt

@@ -1,4 +1 @@
 build
-flake8
-pytest
-black

lint-requirements.txt

@@ -1,2 +1,4 @@
-flake8
-black
+flake8-bugbear < 24.5
+black ~= 24.4
+codespell
+isort >= 5

test/test_client_util.py

@@ -2,12 +2,13 @@
 import os
 import logging
 import subprocess
-import sys
 
 from wes_client.util import expand_globs, wf_info
 
 logging.basicConfig(level=logging.INFO)
 
+PRE = "https://raw.githubusercontent.com/common-workflow-language/workflow-service/main"
+
 
 class IntegrationTest(unittest.TestCase):
     def setUp(self):
@@ -21,10 +22,11 @@ def setUp(self):
         }
 
         self.remote = {
-            "cwl": "https://raw.githubusercontent.com/common-workflow-language/workflow-service/master/testdata/md5sum.cwl",
-            "wdl": "https://raw.githubusercontent.com/common-workflow-language/workflow-service/master/testdata/md5sum.wdl",
-            "py": "https://raw.githubusercontent.com/common-workflow-language/workflow-service/master/test/test_integration.py",
-            "unsupported": "gs://topmed_workflow_testing/topmed_aligner/small_test_files_sbg/example_human_known_snp.py",
+            "cwl": f"{PRE}/testdata/md5sum.cwl",
+            "wdl": f"{PRE}/testdata/md5sum.wdl",
+            "py": f"{PRE}/test/test_integration.py",
+            "unsupported": "gs://topmed_workflow_testing/topmed_aligner/"
+            "small_test_files_sbg/example_human_known_snp.py",
             "unreachable": "https://fake.py",
         }
 
@@ -63,7 +65,8 @@ def testSupportedFormatChecking(self):
 
         for file_format, location in self.local.items():
             if file_format != "unsupported":
-                # Tests the behavior after receiving supported file types with and without the 'file://' prefix
+                # Tests the behavior after receiving supported file types with
+                # and without the 'file://' prefix
                 self.assertEqual(wf_info(location), self.expected[file_format])
                 self.assertEqual(wf_info(location[7:]), self.expected[file_format])
 

test/test_integration.py

@@ -5,13 +5,9 @@
 import signal
 import shutil
 import logging
-import sys
 import requests
 import pytest
 
-pkg_root = os.path.abspath(os.path.join(os.path.dirname(__file__), ".."))  # noqa
-sys.path.insert(0, pkg_root)  # noqa
-
 from wes_client.util import WESClient
 
 logging.basicConfig(level=logging.INFO)
@@ -23,7 +19,11 @@ class IntegrationTest(unittest.TestCase):
     @classmethod
     def setUpClass(cls):
         # cwl
-        cls.cwl_dockstore_url = "https://dockstore.org/api/ga4gh/trs/v2/tools/github.com%2Fmr-c%2Fdockstore-tool-md5sum/versions/master/PLAIN_CWL/descriptor//Dockstore.cwl"
+        cls.cwl_dockstore_url = (
+            "https://dockstore.org/api/ga4gh/trs/v2/tools/"
+            "github.com%2Fmr-c%2Fdockstore-tool-md5sum/versions/"
+            "master/PLAIN_CWL/descriptor//Dockstore.cwl"
+        )
         cls.cwl_local_path = "file://" + os.path.abspath("testdata/md5sum.cwl")
         cls.cwl_json_input = "file://" + os.path.abspath("testdata/md5sum.json")
         cls.cwl_attachments = [
@@ -66,7 +66,7 @@ def test_dockstore_md5sum(self):
             json_input=self.cwl_json_input,
             workflow_attachment=self.cwl_attachments,
         )
-        state = self.wait_for_finish(run_id)
+        self.wait_for_finish(run_id)
         self.check_complete(run_id)
         self.assertTrue(
             self.check_for_file(outfile_path),
@@ -80,7 +80,7 @@ def test_local_md5sum(self):
             json_input=self.cwl_json_input,
             workflow_attachment=self.cwl_attachments,
         )
-        state = self.wait_for_finish(run_id)
+        self.wait_for_finish(run_id)
         self.check_complete(run_id)
         self.assertTrue(
             self.check_for_file(outfile_path),
@@ -99,7 +99,7 @@ def test_run_attachments(self):
             workflow_attachment=self.cwl_attachments,
         )
         get_response = self.client.get_run_log(run_id)["request"]
-        state = self.wait_for_finish(run_id)
+        self.wait_for_finish(run_id)
         self.check_complete(run_id)
         self.assertTrue(
             self.check_for_file(outfile_path),
@@ -150,7 +150,11 @@ def test_get_run_status(self):
         assert "run_id" in r
 
     def run_md5sum(self, wf_input, json_input, workflow_attachment=None):
-        """Pass a local md5sum cwl to the wes-service server, and return the path of the output file that was created."""
+        """
+        Pass a local md5sum cwl to the wes-service server.
+
+        :return: the path of the output file that was created.
+        """
         response = self.client.run(wf_input, json_input, workflow_attachment)
         assert "run_id" in response, str(response.json())
         output_dir = os.path.abspath(

wes_client/util.py

@@ -16,8 +16,8 @@ def py3_compatible(filePath):
     """Determines if a python file is 3.x compatible by seeing if it compiles in a subprocess"""
     try:
         check_call(["python3", "-m", "py_compile", filePath], stderr=DEVNULL)
-    except CalledProcessError:
-        raise RuntimeError("Python files must be 3.x compatible")
+    except CalledProcessError as e:
+        raise RuntimeError("Python files must be 3.x compatible") from e
     return True
 
 
@@ -28,7 +28,8 @@ def get_version(extension, workflow_file):
     elif extension == "cwl":
         return yaml.load(open(workflow_file), Loader=yaml.FullLoader)["cwlVersion"]
     else:  # Must be a wdl file.
-        # Borrowed from https://github.com/Sage-Bionetworks/synapse-orchestrator/blob/develop/synorchestrator/util.py#L142
+        # Borrowed from https://github.com/Sage-Bionetworks/synapse-orchestrator/
+        #               blob/develop/synorchestrator/util.py#L142
         try:
             return [
                 entry.lstrip("version")
@@ -43,8 +44,9 @@ def wf_info(workflow_path):
     """
     Returns the version of the file and the file extension.
 
-    Assumes that the file path is to the file directly ie, ends with a valid file extension.Supports checking local
-    files as well as files at http:// and https:// locations. Files at these remote locations are recreated locally to
+    Assumes that the file path is to the file directly ie, ends with a valid
+    file extension. Supports checking local files as well as files at http://
+    and https:// locations. Files at these remote locations are recreated locally to
     enable our approach to version checking, then removed after version is extracted.
     """
 

wes_client/wes_client_main.py

@@ -23,7 +23,8 @@ def main(argv=sys.argv[1:]):
         "--auth",
         type=str,
         default=os.environ.get("WES_API_AUTH"),
-        help="Format is 'Header: value' or just 'value'.  If header name is not provided, value goes in the 'Authorization'.  Defaults to WES_API_AUTH.",
+        help="Format is 'Header: value' or just 'value'.  If header name is not "
+        "provided, value goes in the 'Authorization'.  Defaults to WES_API_AUTH.",
     )
     parser.add_argument(
         "--proto",

wes_service/arvados_wes.py

@@ -171,7 +171,7 @@ def invoke_cwl_runner(
                 inputtemp.flush()
 
                 msg = ""
-                for dirpath, dirs, files in os.walk(tempdir):
+                for dirpath, _dirs, files in os.walk(tempdir):
                     for f in files:
                         msg += "  " + dirpath + "/" + f + "\n"
 

wes_service/cwl_runner.py

@@ -26,7 +26,8 @@ def run(self, request, tempdir, opts):
         CWL (url):
         request["workflow_url"] == a url to a cwl file
         or
-        request["workflow_attachment"] == input cwl text (written to a file and a url constructed for that file)
+        request["workflow_attachment"] == input cwl text
+        (written to a file and a url constructed for that file)
 
         JSON File:
         request["workflow_params"] == input json text (to be written to a file)
@@ -53,10 +54,11 @@ def run(self, request, tempdir, opts):
         extra = opts.getoptlist("extra")
 
         # replace any locally specified outdir with the default
+        extra2 = []
         for e in extra:
-            if e.startswith("--outdir="):
-                extra.remove(e)
-        extra.append("--outdir=" + self.outdir)
+            if not e.startswith("--outdir="):
+                extra2.append(e)
+        extra2.append("--outdir=" + self.outdir)
 
         # link the cwl and json into the tempdir/cwd
         if workflow_url.startswith("file://"):
@@ -66,7 +68,7 @@ def run(self, request, tempdir, opts):
         jsonpath = os.path.join(tempdir, "cwl.input.json")
 
         # build args and run
-        command_args = [runner] + extra + [workflow_url, jsonpath]
+        command_args = [runner] + extra2 + [workflow_url, jsonpath]
         proc = subprocess.Popen(
             command_args, stdout=output, stderr=stderr, close_fds=True, cwd=tempdir
         )

wes_service/toil_wes.py

@@ -45,24 +45,25 @@ def __init__(self, run_id):
     def sort_toil_options(self, extra):
         # determine jobstore and set a new default if the user did not set one
         cloud = False
+        extra2 = []
         for e in extra:
             if e.startswith("--jobStore="):
                 self.jobstore = e[11:]
                 if self.jobstore.startswith(("aws", "google", "azure")):
                     cloud = True
-            if e.startswith(("--outdir=", "-o=")):
-                extra.remove(e)
+            if not e.startswith(("--outdir=", "-o=")):
+                extra2.append(e)
         if not cloud:
-            extra.append("--outdir=" + self.outdir)
+            extra2.append("--outdir=" + self.outdir)
         if not self.jobstore:
-            extra.append("--jobStore=" + self.jobstore_default)
+            extra2.append("--jobStore=" + self.jobstore_default)
             self.jobstore = self.jobstore_default
 
         # store the jobstore location
         with open(self.jobstorefile, "w") as f:
             f.write(self.jobstore)
 
-        return extra
+        return extra2
 
     def write_workflow(self, request, opts, cwd, wftype="cwl"):
         """Writes a cwl, wdl, or python file as appropriate from the request dictionary."""
@@ -199,7 +200,8 @@ def run(self, request, tempdir, opts):
         CWL (url):
         request["workflow_url"] == a url to a cwl file
         or
-        request["workflow_attachment"] == input cwl text (written to a file and a url constructed for that file)
+        request["workflow_attachment"] == input cwl text
+        (written to a file and a url constructed for that file)
 
         JSON File:
         request["workflow_params"] == input json text (to be written to a file)

wes_service/util.py

@@ -64,7 +64,7 @@ def collect_attachments(self, run_id=None):
                             os.makedirs(os.path.dirname(dest))
                         self.log_for_run(
                             run_id,
-                            f"Staging attachment '{v.filename}' to '{dest}'",
+                            f"Staging attachment {v.filename!r} to {dest!r}",
                         )
                         v.save(dest)
                         has_attachments = True
@@ -77,7 +77,7 @@ def collect_attachments(self, run_id=None):
                     else:
                         body[k] = v.read().decode()
             except Exception as e:
-                raise ValueError(f"Error reading parameter '{k}': {e}")
+                raise ValueError(f"Error reading parameter {k!r}: {e}") from e
         for k, ls in connexion.request.form.lists():
             try:
                 for v in ls:
@@ -88,7 +88,7 @@ def collect_attachments(self, run_id=None):
                     else:
                         body[k] = v
             except Exception as e:
-                raise ValueError(f"Error reading parameter '{k}': {e}")
+                raise ValueError(f"Error reading parameter {k!r}: {e}") from e
 
         if "workflow_url" in body:
             if ":" not in body["workflow_url"]: