Skip to content

Commit

Permalink
allow subworkflow name in input JSON key
Browse files Browse the repository at this point in the history
To supply an optional input for a subworkflow call, Cromwell input JSON expects the key to include (i) the workflow name, (ii) the call name, (iii) the subworkflow name, and finally (iv) the input name. The need to include (iii) the subworkflow name is questionable, as discussed on openwdl/wdl#340

This change makes miniwdl accept such inputs either with or without the subworkflow name, by removing it if it's present. To ensure unambiguous resolution, also forbids collision between a call's name and that of its containing workflow.

#193
  • Loading branch information
mlin authored Dec 17, 2019
1 parent c35ded4 commit daa5a27
Show file tree
Hide file tree
Showing 7 changed files with 41 additions and 12 deletions.
6 changes: 1 addition & 5 deletions WDL/Lint.py
Original file line number Diff line number Diff line change
Expand Up @@ -478,7 +478,6 @@ def call(self, obj: Tree.Call) -> Any:
class NameCollision(Linter):
# Name collisions between
# - call and import
# - call and its containing workflow
# - call and struct type/alias
# - decl and import
# - decl and workflow
Expand All @@ -503,13 +502,10 @@ def call(self, obj: Tree.Call) -> Any:
if imp.namespace == obj.name:
msg = "call name '{}' collides with imported document namespace".format(obj.name)
self.add(obj, msg)
if doc.workflow and doc.workflow.name == obj.name:
msg = "call name '{}' collides with workflow name".format(obj.name)
self.add(obj, msg)
for stb in doc.struct_typedefs:
assert isinstance(stb, Env.Binding) and isinstance(stb.value, Tree.StructTypeDef)
if stb.name == obj.name:
msg = "call name '{}' colides with {}struct type".format(
msg = "call name '{}' collides with {}struct type".format(
obj.name, "imported " if stb.value.imported else ""
)
self.add(obj, msg)
Expand Down
5 changes: 5 additions & 0 deletions WDL/Tree.py
Original file line number Diff line number Diff line change
Expand Up @@ -483,6 +483,11 @@ def resolve(self, doc: "Document") -> None:
self.callee = task
if self.callee is None:
raise Error.NoSuchTask(self, ".".join(self.callee_id))
assert doc.workflow
if self.name == doc.workflow.name:
raise Error.MultipleDefinitions(
self, "Call's name may not equal the containing workflow's"
)
assert isinstance(self.callee, (Task, Workflow))

def add_to_type_env(
Expand Down
5 changes: 5 additions & 0 deletions WDL/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,11 @@ def values_from_json(
key2 = key
if namespace and key.startswith(namespace):
key2 = key[len(namespace) :]
if key2 not in available:
# attempt to simplify <call>.<subworkflow>.<input>
key2parts = key2.split(".")
if len(key2parts) == 3 and key2parts[0] and key2parts[1] and key2parts[2]:
key2 = ".".join([key2parts[0], key2parts[2]])
try:
ty = available[key2]
except KeyError:
Expand Down
6 changes: 3 additions & 3 deletions test_corpi/contrived/contrived.wdl
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ workflow contrived {
Float required
}
Int? fallaciously_optional = 123
call popular as contrived { input:
call popular as c1 { input:
popular = popular,
i = fortytwo,
y = [select_first([fortytwo,23])]
Expand All @@ -28,8 +28,8 @@ workflow contrived {
Pair[Pair[String,String],Pair[Int,Int]] p2 = ((c2.left_contents, c2.right_contents), (4,2))

output {
Int read_int = read_json(contrived.json) + p2.right.left + p2.right.right
Array[Boolean] read_array = read_json(contrived.json)
Int read_int = read_json(c1.json) + p2.right.left + p2.right.right
Array[Boolean] read_array = read_json(c1.json)
String left_contents = p2.left.left
String right_contents = p2.left.right
}
Expand Down
13 changes: 11 additions & 2 deletions tests/test_2calls.py
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,15 @@ def test_collision(self):
with self.assertRaises(WDL.Error.MultipleDefinitions):
doc.typecheck()

txt = task_no_outputs + r"""
workflow contrived {
call p as contrived
}
"""
doc = WDL.parse_document(txt)
with self.assertRaises(WDL.Error.MultipleDefinitions):
doc.typecheck()

def test_if_defined(self):
# test how we typecheck a construct like
# if defined(x) then EXPR_WITH_x else SOME_DEFAULT
Expand Down Expand Up @@ -497,8 +506,8 @@ def test_io_propagation(self):
doc.workflow.available_inputs.resolve("popular")
doc.workflow.available_inputs.resolve("fortytwo")
doc.workflow.available_inputs.resolve("required")
self.assertTrue(doc.workflow.available_inputs.has_namespace("contrived"))
doc.workflow.available_inputs.resolve("contrived.opt")
self.assertTrue(doc.workflow.available_inputs.has_namespace("c1"))
doc.workflow.available_inputs.resolve("c1.opt")
self.assertTrue(doc.workflow.available_inputs.has_namespace("c2"))
doc.workflow.available_inputs.enter_namespace("c2").resolve("opt")
doc.workflow.available_inputs.enter_namespace("c2").resolve("i")
Expand Down
4 changes: 2 additions & 2 deletions tests/test_3corpi.py
Original file line number Diff line number Diff line change
Expand Up @@ -247,15 +247,15 @@ class dxWDL(unittest.TestCase):

@wdl_corpus(
["test_corpi/contrived/**"],
expected_lint={'UnusedImport': 4, 'NameCollision': 30, 'StringCoercion': 4, 'FileCoercion': 2, 'NonemptyCoercion': 1, 'UnnecessaryQuantifier': 2, 'UnusedDeclaration': 2, "IncompleteCall": 2, 'SelectArray': 1},
expected_lint={'UnusedImport': 4, 'NameCollision': 27, 'StringCoercion': 4, 'FileCoercion': 2, 'NonemptyCoercion': 1, 'UnnecessaryQuantifier': 2, 'UnusedDeclaration': 2, "IncompleteCall": 2, 'SelectArray': 1},
blacklist=["check_quant", "incomplete_call"],
)
class Contrived(unittest.TestCase):
pass

@wdl_corpus(
["test_corpi/contrived/**"],
expected_lint={'UnusedImport': 6, 'NameCollision': 49, 'StringCoercion': 9, 'FileCoercion': 4, 'OptionalCoercion': 3, 'NonemptyCoercion': 2, 'UnnecessaryQuantifier': 4, 'UnusedDeclaration': 9, 'IncompleteCall': 3, 'ArrayCoercion': 2, 'SelectArray': 4},
expected_lint={'UnusedImport': 6, 'NameCollision': 43, 'StringCoercion': 9, 'FileCoercion': 4, 'OptionalCoercion': 3, 'NonemptyCoercion': 2, 'UnnecessaryQuantifier': 4, 'UnusedDeclaration': 9, 'IncompleteCall': 3, 'ArrayCoercion': 2, 'SelectArray': 4},
check_quant=False,
blacklist=["incomplete_call"],
)
Expand Down
14 changes: 14 additions & 0 deletions tests/test_6workflowrun.py
Original file line number Diff line number Diff line change
Expand Up @@ -580,6 +580,20 @@ def test_subworkflow(self):
self.assertEqual(outputs["sums"], [1, 5, 14])
self.assertEqual(outputs["sum"], 20)

subwf_input = R"""
version 1.0
import "sum_sq.wdl" as lib
workflow subwf_input {
call lib.sum_sq as summer
output {
Int ans = summer.ans
}
}
"""
self._test_workflow(subwf_input, {"summer.n": 3})
self._test_workflow(subwf_input, {"summer.sum_sq.n": 3})

def test_host_file_access(self):
exn = self._test_workflow("""
version 1.0
Expand Down

0 comments on commit daa5a27

Please sign in to comment.