flake8 cleanup

Bonus type modernizing and some docstrings

Author: mr-c

.flake8

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

cwltool/argparser.py

@@ -42,7 +42,8 @@ def arg_parser() -> argparse.ArgumentParser:
         type=str,
         default="",
         help="Log your tools stdout/stderr to this location outside of container "
-        "This will only log stdout/stderr if you specify stdout/stderr in their respective fields or capture it as an output",
+        "This will only log stdout/stderr if you specify stdout/stderr in their "
+        "respective fields or capture it as an output",
     )
 
     parser.add_argument(
@@ -741,7 +742,7 @@ def get_default_args() -> Dict[str, Any]:
 
 
 class FSAction(argparse.Action):
-    objclass = None  # type: str
+    objclass: Optional[str] = None
 
     def __init__(
         self,
@@ -777,7 +778,7 @@ def __call__(
 
 
 class FSAppendAction(argparse.Action):
-    objclass = None  # type: str
+    objclass: Optional[str] = None
 
     def __init__(
         self,
@@ -815,19 +816,19 @@ def __call__(
 
 
 class FileAction(FSAction):
-    objclass = "File"
+    objclass: Optional[str] = "File"
 
 
 class DirectoryAction(FSAction):
-    objclass = "Directory"
+    objclass: Optional[str] = "Directory"
 
 
 class FileAppendAction(FSAppendAction):
-    objclass = "File"
+    objclass: Optional[str] = "File"
 
 
 class DirectoryAppendAction(FSAppendAction):
-    objclass = "Directory"
+    objclass: Optional[str] = "Directory"
 
 
 class AppendAction(argparse.Action):
@@ -900,9 +901,9 @@ def add_argument(
             return None
 
     ahelp = description.replace("%", "%%")
-    action = None  # type: Optional[Union[Type[argparse.Action], str]]
-    atype = None  # type: Any
-    typekw = {}  # type: Dict[str, Any]
+    action: Optional[Union[Type[argparse.Action], str]] = None
+    atype: Optional[Any] = None
+    typekw: Dict[str, Any] = {}
 
     if inptype == "File":
         action = FileAction

cwltool/builder.py

@@ -2,29 +2,31 @@
 import copy
 import logging
 import math
-from typing import (
+from typing import (  # pylint: disable=unused-import
     IO,
+    TYPE_CHECKING,
     Any,
     Callable,
     Dict,
     List,
     MutableMapping,
     MutableSequence,
     Optional,
+    Type,
     Union,
     cast,
 )
 
 from cwl_utils import expression
 from cwl_utils.file_formats import check_format
 from rdflib import Graph
-from ruamel.yaml.comments import CommentedMap
 from schema_salad.avro.schema import Names, Schema, make_avsc_object
 from schema_salad.exceptions import ValidationException
 from schema_salad.sourceline import SourceLine
 from schema_salad.utils import convert_to_dict, json_dumps
 from schema_salad.validate import validate
-from typing_extensions import TYPE_CHECKING, Type  # pylint: disable=unused-import
+
+from ruamel.yaml.comments import CommentedMap
 
 from .errors import WorkflowException
 from .loghandler import _logger
@@ -186,10 +188,8 @@ def bind_input(
         if lead_pos is None:
             lead_pos = []
 
-        bindings = []  # type: List[MutableMapping[str, Union[str, List[int]]]]
-        binding = (
-            {}
-        )  # type: Union[MutableMapping[str, Union[str, List[int]]], CommentedMap]
+        bindings: List[MutableMapping[str, Union[str, List[int]]]] = []
+        binding: Union[MutableMapping[str, Union[str, List[int]]], CommentedMap] = {}
         value_from_expression = False
         if "inputBinding" in schema and isinstance(
             schema["inputBinding"], MutableMapping
@@ -208,7 +208,7 @@ def bind_input(
                         ).makeError(
                             "'position' expressions must evaluate to an int, "
                             f"not a {type(result)}. Expression {position} "
-                            f"resulted in '{result}'."
+                            f"resulted in {result!r}."
                         )
                     binding["position"] = result
                     bp.append(result)
@@ -227,7 +227,7 @@ def bind_input(
         if isinstance(schema["type"], MutableSequence):
             bound_input = False
             for t in schema["type"]:
-                avsc = None  # type: Optional[Schema]
+                avsc: Optional[Schema] = None
                 if isinstance(t, str) and self.names.has_name(t, None):
                     avsc = self.names.get_name(t, None)
                 elif (
@@ -336,10 +336,10 @@ def bind_input(
                     if binding:
                         b2 = cast(CWLObjectType, copy.deepcopy(binding))
                         b2["datum"] = item
-                    itemschema = {
+                    itemschema: CWLObjectType = {
                         "type": schema["items"],
                         "inputBinding": b2,
-                    }  # type: CWLObjectType
+                    }
                     for k in ("secondaryFiles", "format", "streamable"):
                         if k in schema:
                             itemschema[k] = schema[k]
@@ -362,9 +362,9 @@ def _capture_files(f: CWLObjectType) -> CWLObjectType:
                 datum = cast(CWLObjectType, datum)
                 self.files.append(datum)
 
-                loadContents_sourceline = (
-                    None
-                )  # type: Union[None, MutableMapping[str, Union[str, List[int]]], CWLObjectType]
+                loadContents_sourceline: Union[
+                    None, MutableMapping[str, Union[str, List[int]]], CWLObjectType
+                ] = None
                 if binding and binding.get("loadContents"):
                     loadContents_sourceline = binding
                 elif schema.get("loadContents"):
@@ -385,7 +385,7 @@ def _capture_files(f: CWLObjectType) -> CWLObjectType:
                         except Exception as e:
                             raise Exception(
                                 "Reading {}\n{}".format(datum["location"], e)
-                            )
+                            ) from e
 
                 if "secondaryFiles" in schema:
                     if "secondaryFiles" not in datum:
@@ -415,8 +415,8 @@ def _capture_files(f: CWLObjectType) -> CWLObjectType:
                                     "The result of a expression in the field "
                                     "'required' must "
                                     f"be a bool or None, not a {type(required_result)}. "
-                                    f"Expression '{sf_entry['required']}' resulted "
-                                    f"in '{required_result}'."
+                                    f"Expression {sf_entry['required']!r} resulted "
+                                    f"in {required_result!r}."
                                 )
                             sf_required = required_result
                         else:
@@ -454,7 +454,7 @@ def _capture_files(f: CWLObjectType) -> CWLObjectType:
                                     "Expected secondaryFile expression to "
                                     "return type 'str', a 'File' or 'Directory' "
                                     "dictionary, or a list of the same. Received "
-                                    f"'{type(sfname)} from '{sf_entry['pattern']}'."
+                                    f"{type(sfname)!r} from {sf_entry['pattern']!r}."
                                 )
 
                             for d in cast(
@@ -529,9 +529,9 @@ def addsf(
                                     "An expression in the 'format' field must "
                                     "evaluate to a string, or list of strings. "
                                     "However a non-string item was received: "
-                                    f"'{entry}' of type '{type(entry)}'. "
-                                    f"The expression was '{schema['format']}' and "
-                                    f"its fully evaluated result is '{eval_format}'."
+                                    f"{entry!r} of type {type(entry)!r}. "
+                                    f"The expression was {schema['format']!r} and "
+                                    f"its fully evaluated result is {eval_format!r}."
                                 )
                             if expression.needs_parsing(entry):
                                 message = (
@@ -542,7 +542,7 @@ def addsf(
                                     "Expressions or CWL Parameter References. List "
                                     f"entry number {index+1} contains the following "
                                     "unallowed CWL Parameter Reference or Expression: "
-                                    f"'{entry}'."
+                                    f"{entry!r}."
                                 )
                             if message:
                                 raise SourceLine(
@@ -557,8 +557,8 @@ def addsf(
                             "evaluate to a string, or list of strings. "
                             "However the type of the expression result was "
                             f"{type(eval_format)}. "
-                            f"The expression was '{schema['format']}' and "
-                            f"its fully evaluated result is 'eval_format'."
+                            f"The expression was {schema['format']!r} and "
+                            f"its fully evaluated result is {eval_format!r}."
                         )
                     try:
                         check_format(
@@ -568,8 +568,8 @@ def addsf(
                         )
                     except ValidationException as ve:
                         raise WorkflowException(
-                            "Expected value of '%s' to have format %s but\n "
-                            " %s" % (schema["name"], schema["format"], ve)
+                            f"Expected value of {schema['name']!r} to have "
+                            f"format {schema['format']!r} but\n {ve}"
                         ) from ve
 
                 visit_class(
@@ -646,7 +646,7 @@ def generate_arg(self, binding: CWLObjectType) -> List[str]:
                     "'separate' option can not be specified without prefix"
                 )
 
-        argl = []  # type: MutableSequence[CWLOutputType]
+        argl: MutableSequence[CWLOutputType] = []
         if isinstance(value, MutableSequence):
             if binding.get("itemSeparator") and value:
                 itemSeparator = cast(str, binding["itemSeparator"])

cwltool/checker.py

@@ -62,7 +62,7 @@ def check_types(
         return check_types(
             merge_flatten_type(_get_type(srctype)), _get_type(sinktype), None, None
         )
-    raise WorkflowException(f"Unrecognized linkMerge enum '{linkMerge}'")
+    raise WorkflowException(f"Unrecognized linkMerge enum {linkMerge!r}")
 
 
 def merge_flatten_type(src: SinkType) -> CWLOutputType:
@@ -561,8 +561,8 @@ def loop_checker(steps: Iterator[MutableMapping[str, Any]]) -> None:
     """
     Check http://commonwl.org/cwltool#Loop requirement compatibility with other directives.
 
-    :raises:
-        ValidationException: If there is an incompatible combination between cwltool:loop and 'scatter' or 'when'.
+    :raises ValidationException: If there is an incompatible combination between
+            cwltool:loop and 'scatter' or 'when'.
     """
     exceptions = []
     for step in steps: