Skip to content

Commit 592a7e6

Browse files
mr-cmr-c
authored
Merge pull request #942 from psafont/check-revamp
Use explicits checks when possible
2 parents c3f7a56 + e94c818 commit 592a7e6

28 files changed

+951
-474
lines changed

‎cwltool/argparser.py

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -146,13 +146,13 @@ def arg_parser(): # type: () -> argparse.ArgumentParser
146146
"--orcid", help="Record user ORCID identifier as part of "
147147
"provenance, e.g. https://orcid.org/0000-0002-1825-0097 "
148148
"or 0000-0002-1825-0097. Alternatively the environment variable "
149-
"ORCID may be set.", dest="orcid", default=os.environ.get("ORCID"),
149+
"ORCID may be set.", dest="orcid", default=os.environ.get("ORCID", ''),
150150
type=Text)
151151
provgroup.add_argument(
152152
"--full-name", help="Record full name of user as part of provenance, "
153153
"e.g. Josiah Carberry. You may need to use shell quotes to preserve "
154154
"spaces. Alternatively the environment variable CWL_FULL_NAME may "
155-
"be set.", dest="cwl_full_name", default=os.environ.get("CWL_FULL_NAME"),
155+
"be set.", dest="cwl_full_name", default=os.environ.get("CWL_FULL_NAME", ''),
156156
type=Text)
157157

158158
exgroup = parser.add_mutually_exclusive_group()
@@ -381,7 +381,7 @@ def add_argument(toolparser, name, inptype, records, description="",
381381
else:
382382
flag = "--"
383383

384-
required = True
384+
required = default is None
385385
if isinstance(inptype, MutableSequence):
386386
if inptype[0] == "null":
387387
required = False
@@ -418,7 +418,7 @@ def add_argument(toolparser, name, inptype, records, description="",
418418
toolparser, fieldname, fieldtype, records,
419419
fielddescription)
420420
return
421-
if inptype == "string":
421+
elif inptype == "string":
422422
atype = Text
423423
elif inptype == "int":
424424
atype = int
@@ -428,11 +428,7 @@ def add_argument(toolparser, name, inptype, records, description="",
428428
atype = float
429429
elif inptype == "boolean":
430430
action = "store_true"
431-
432-
if default:
433-
required = False
434-
435-
if not atype and not action:
431+
else:
436432
_logger.debug(u"Can't make command line argument from %s", inptype)
437433
return None
438434

‎cwltool/builder.py

Lines changed: 50 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -97,8 +97,8 @@ def __init__(self):
9797
self.hints = [] # List[Dict[Text, Any]]
9898

9999
def get_requirement(self,
100-
feature # type: Text
101-
): # type: (...) -> Tuple[Optional[Any], Optional[bool]]
100+
feature # type: Text
101+
): # type: (...) -> Tuple[Optional[Any], Optional[bool]]
102102
for item in reversed(self.requirements):
103103
if item["class"] == feature:
104104
return (item, True)
@@ -109,93 +109,68 @@ def get_requirement(self,
109109

110110
class Builder(HasReqsHints):
111111
def __init__(self,
112-
job, # type: Dict[Text, Union[Dict[Text, Any], List, Text, None]]
113-
files=None, # type: List[Dict[Text, Text]]
114-
bindings=None, # type: List[Dict[Text, Any]]
115-
schemaDefs=None, # type: Dict[Text, Dict[Text, Any]]
116-
names=None, # type: Names
117-
requirements=None, # type: List[Dict[Text, Any]]
118-
hints=None, # type: List[Dict[Text, Any]]
119-
timeout=None, # type: float
120-
debug=False, # type: bool
121-
resources=None, # type: Dict[str, int]
122-
js_console=False, # type: bool
123-
mutation_manager=None, # type: Optional[MutationManager]
124-
formatgraph=None, # type: Optional[Graph]
125-
make_fs_access=None, # type: Type[StdFsAccess]
126-
fs_access=None, # type: StdFsAccess
127-
force_docker_pull=False, # type: bool
128-
loadListing=u"", # type: Text
129-
outdir=u"", # type: Text
130-
tmpdir=u"", # type: Text
131-
stagedir=u"", # type: Text
132-
job_script_provider=None # type: Optional[Any]
112+
job, # type: Dict[Text, Union[Dict[Text, Any], List, Text, None]]
113+
files, # type: List[Dict[Text, Text]]
114+
bindings, # type: List[Dict[Text, Any]]
115+
schemaDefs, # type: Dict[Text, Dict[Text, Any]]
116+
names, # type: Names
117+
requirements, # type: List[Dict[Text, Any]]
118+
hints, # type: List[Dict[Text, Any]]
119+
resources, # type: Dict[str, int]
120+
mutation_manager, # type: Optional[MutationManager]
121+
formatgraph, # type: Optional[Graph]
122+
make_fs_access, # type: Type[StdFsAccess]
123+
fs_access, # type: StdFsAccess
124+
job_script_provider, # type: Optional[Any]
125+
timeout, # type: float
126+
debug, # type: bool
127+
js_console, # type: bool
128+
force_docker_pull, # type: bool
129+
loadListing, # type: Text
130+
outdir, # type: Text
131+
tmpdir, # type: Text
132+
stagedir, # type: Text
133133
): # type: (...) -> None
134134

135-
if names is None:
136-
self.names = Names()
137-
else:
138-
self.names = names
139-
140-
if schemaDefs is None:
141-
self.schemaDefs = {} # type: Dict[Text, Dict[Text, Any]]
142-
else:
143-
self.schemaDefs = schemaDefs
144-
145-
if files is None:
146-
self.files = [] # type: List[Dict[Text, Text]]
147-
else:
148-
self.files = files
149-
150135
self.job = job
136+
self.files = files
137+
self.bindings = bindings
138+
self.schemaDefs = schemaDefs
139+
self.names = names
151140
self.requirements = requirements
152141
self.hints = hints
153-
self.outdir = outdir
154-
self.tmpdir = tmpdir
155-
156-
if resources is None:
157-
self.resources = {} # type: Dict[str, int]
158-
else:
159-
self.resources = resources
142+
self.resources = resources
143+
self.mutation_manager = mutation_manager
144+
self.formatgraph = formatgraph
160145

161-
if bindings is None:
162-
self.bindings = [] # type: List[Dict[Text, Any]]
163-
else:
164-
self.bindings = bindings
165-
self.timeout = timeout
166-
self.pathmapper = None # type: Optional[PathMapper]
167-
self.stagedir = stagedir
146+
self.make_fs_access = make_fs_access
147+
self.fs_access = fs_access
168148

169-
if make_fs_access is None:
170-
self.make_fs_access = StdFsAccess
171-
else:
172-
self.make_fs_access = make_fs_access
149+
self.job_script_provider = job_script_provider
173150

174-
if fs_access is None:
175-
self.fs_access = self.make_fs_access("")
176-
else:
177-
self.fs_access = fs_access
151+
self.timeout = timeout
178152

179153
self.debug = debug
180154
self.js_console = js_console
181-
self.mutation_manager = mutation_manager
182155
self.force_docker_pull = force_docker_pull
183-
self.formatgraph = formatgraph
184156

185157
# One of "no_listing", "shallow_listing", "deep_listing"
186158
self.loadListing = loadListing
187-
self.prov_obj = None # type: Optional[CreateProvProfile]
188159

160+
self.outdir = outdir
161+
self.tmpdir = tmpdir
162+
self.stagedir = stagedir
163+
164+
self.pathmapper = None # type: Optional[PathMapper]
165+
self.prov_obj = None # type: Optional[CreateProvProfile]
189166
self.find_default_container = None # type: Optional[Callable[[], Text]]
190-
self.job_script_provider = job_script_provider
191167

192168
def build_job_script(self, commands):
193169
# type: (List[Text]) -> Text
194170
build_job_script_method = getattr(self.job_script_provider, "build_job_script", None) # type: Callable[[Builder, Union[List[str],List[Text]]], Text]
195-
if build_job_script_method:
171+
if build_job_script_method is not None:
196172
return build_job_script_method(self, commands)
197-
else:
198-
return None
173+
return None
199174

200175
def bind_input(self,
201176
schema, # type: MutableMapping[Text, Any]
@@ -209,6 +184,7 @@ def bind_input(self,
209184
tail_pos = []
210185
if lead_pos is None:
211186
lead_pos = []
187+
212188
bindings = [] # type: List[MutableMapping[Text, Text]]
213189
binding = None # type: Optional[MutableMapping[Text,Any]]
214190
value_from_expression = False
@@ -247,7 +223,11 @@ def bind_input(self,
247223
raise validate.ValidationException(u"'%s' is not a valid union %s" % (datum, schema["type"]))
248224
elif isinstance(schema["type"], MutableMapping):
249225
st = copy.deepcopy(schema["type"])
250-
if binding and "inputBinding" not in st and st["type"] == "array" and "itemSeparator" not in binding:
226+
if binding is not None\
227+
and "inputBinding" not in st\
228+
and "type" in st\
229+
and st["type"] == "array"\
230+
and "itemSeparator" not in binding:
251231
st["inputBinding"] = {}
252232
for k in ("secondaryFiles", "format", "streamable"):
253233
if k in schema:
@@ -270,7 +250,7 @@ def bind_input(self,
270250
if schema["type"] == "array":
271251
for n, item in enumerate(datum):
272252
b2 = None
273-
if binding:
253+
if binding is not None:
274254
b2 = copy.deepcopy(binding)
275255
b2["datum"] = item
276256
itemschema = {
@@ -341,7 +321,7 @@ def _capture_files(f):
341321
self.files.append(datum)
342322

343323
# Position to front of the sort key
344-
if binding:
324+
if binding is not None:
345325
for bi in bindings:
346326
bi["position"] = binding["position"] + bi["position"]
347327
bindings.append(binding)

‎cwltool/checker.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,9 @@ def check_types(srctype, sinktype, linkMerge, valueFrom):
2626
"""Check if the source and sink types are "pass", "warning", or "exception".
2727
"""
2828

29-
if valueFrom:
29+
if valueFrom is not None:
3030
return "pass"
31-
if not linkMerge:
31+
if linkMerge is None:
3232
if can_assign_src_to_sink(srctype, sinktype, strict=True):
3333
return "pass"
3434
if can_assign_src_to_sink(srctype, sinktype, strict=False):
@@ -39,7 +39,7 @@ def check_types(srctype, sinktype, linkMerge, valueFrom):
3939
_get_type(sinktype), None, None)
4040
if linkMerge == "merge_flattened":
4141
return check_types(merge_flatten_type(_get_type(srctype)), _get_type(sinktype), None, None)
42-
raise WorkflowException(u"Unrecognized linkMerge enu_m '{}'".format(linkMerge))
42+
raise WorkflowException(u"Unrecognized linkMerge enum '{}'".format(linkMerge))
4343

4444

4545
def merge_flatten_type(src):
@@ -179,7 +179,7 @@ def static_checker(workflow_inputs, workflow_outputs, step_inputs, step_outputs,
179179
SourceLine(sink, "type").makeError(
180180
" with sink '%s' of type %s"
181181
% (shortname(sink["id"]), json_dumps(sink["type"])))
182-
if linkMerge:
182+
if linkMerge is not None:
183183
msg += "\n" + SourceLine(sink).makeError(" source has linkMerge method %s" % linkMerge)
184184

185185
warning_msgs.append(msg)
@@ -193,7 +193,7 @@ def static_checker(workflow_inputs, workflow_outputs, step_inputs, step_outputs,
193193
SourceLine(sink, "type").makeError(
194194
" with sink '%s' of type %s"
195195
% (shortname(sink["id"]), json_dumps(sink["type"])))
196-
if linkMerge:
196+
if linkMerge is not None:
197197
msg += "\n" + SourceLine(sink).makeError(" source has linkMerge method %s" % linkMerge)
198198
exception_msgs.append(msg)
199199

0 commit comments

Comments
 (0)