Skip to content

Fix spurious validation error with scatter+conditionals #1640

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Fix spurious validation error with scatter+conditionals #1640

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5810ce6
Fix spurious validation error #1639
tetron Mar 7, 2022
c4442a8
Merge branch 'main' into workflow-checker
tetron Mar 7, 2022
21da544
fix typing
mr-c Mar 8, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 0 additions & 21 deletions cwltool/checker.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -406,27 +406,6 @@ def check_all_types(
)
)

if is_conditional_step(param_to_step, parm_id):
src_typ = aslist(srcs_of_sink[0]["type"])
snk_typ = sink["type"]

if "null" not in src_typ:
src_typ = ["null"] + cast(List[Any], src_typ)

if "null" not in cast(
Union[List[str], CWLObjectType], snk_typ
): # Given our type names this works even if not a list
validation["warning"].append(
SrcSink(
src_dict[parm_id],
sink,
linkMerge,
message="Source is from conditional step and may produce `null`",
)
)

srcs_of_sink[0]["type"] = src_typ

for src in srcs_of_sink:
check_result = check_types(src, sink, linkMerge, valueFrom)
if check_result == "warning":
Expand Down
15 changes: 14 additions & 1 deletion cwltool/workflow.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,8 @@ def __init__(
"SubworkflowFeatureRequirement not in requirements"
)

conditional = self.tool.get("when")

if "scatter" in self.tool:
(feature, _) = self.get_requirement("ScatterFeatureRequirement")
if not feature:
Expand Down Expand Up @@ -398,11 +400,22 @@ def __init__(
else:
nesting = 1

for _ in range(0, nesting):
for n in range(0, nesting):
for oparam in outputparms:
if conditional and n == 0:
oparam["type"] = ["null"] + cast(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a test case that triggers this line?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I was going to add a test, I just wanted to see what the other code checks had to say about it.

List[str], aslist(oparam["type"])
)
oparam["type"] = {"type": "array", "items": oparam["type"]}
self.tool["inputs"] = inputparms
self.tool["outputs"] = outputparms
elif conditional:
# No scatter, but conditional
outputparms = copy.deepcopy(self.tool["outputs"])
for oparam in outputparms:
oparam["type"] = ["null"] + cast(List[str], aslist(oparam["type"]))
self.tool["outputs"] = outputparms

self.prov_obj = None # type: Optional[ProvenanceProfile]
if loadingContext.research_obj is not None:
self.prov_obj = parentworkflowProv
Expand Down