-
Notifications
You must be signed in to change notification settings - Fork 359
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.
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
WX-964 suffix() #7363
WX-964 suffix() #7363
Changes from 4 commits
d1853b1
ad86f77
6fa997c
09f697c
84cdfe4
26b9f4d
c23c3d9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
name: suffix | ||
testFormat: workflowsuccess | ||
|
||
files { | ||
workflow: suffix/suffix.wdl | ||
# https://github.com/broadinstitute/cromwell/issues/4590 | ||
options: suffix/suffix.options | ||
} | ||
|
||
metadata { | ||
workflowName: suffix | ||
status: Succeeded | ||
"outputs.suffix.out": "logs.txt cmd.txt output.txt" | ||
"calls.suffix.sfx.commandLine": "echo \"logs.txt cmd.txt output.txt\"" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
{ | ||
"read_from_cache": false | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
task sfx { | ||
Array[String] filenames = ["logs", "cmd", "output"] | ||
Array[String] suffixed = suffix(".txt", filenames) | ||
command { | ||
echo "${sep=' ' suffixed}" | ||
} | ||
output { | ||
String out = read_string(stdout()) | ||
} | ||
runtime { docker: "ubuntu:latest" } | ||
} | ||
|
||
workflow suffix { | ||
call sfx | ||
output { | ||
String out = sfx.out | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -217,4 +217,22 @@ | |
EvaluatedValue(WomString(arr.value.map(v => v.valueString).mkString(sepvalue.value)), Seq.empty).validNel | ||
} | ||
} | ||
|
||
implicit val suffixFunctionEvaluator: ValueEvaluator[Suffix] = new ValueEvaluator[Suffix] { | ||
Check warning on line 221 in wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluators.scala Codecov / codecov/patchwdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluators.scala#L221
|
||
override def evaluateValue(a: Suffix, | ||
inputs: Map[String, WomValue], | ||
ioFunctionSet: IoFunctionSet, | ||
forCommandInstantiationOptions: Option[ForCommandInstantiationOptions] | ||
)(implicit expressionValueEvaluator: ValueEvaluator[ExpressionElement]): ErrorOr[EvaluatedValue[WomArray]] = | ||
processTwoValidatedValues[WomString, WomArray, WomArray]( | ||
expressionValueEvaluator.evaluateValue(a.arg1, inputs, ioFunctionSet, forCommandInstantiationOptions)( | ||
Check warning on line 228 in wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluators.scala Codecov / codecov/patchwdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluators.scala#L228
|
||
expressionValueEvaluator | ||
), | ||
expressionValueEvaluator.evaluateValue(a.arg2, inputs, ioFunctionSet, forCommandInstantiationOptions)( | ||
Check warning on line 231 in wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluators.scala Codecov / codecov/patchwdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluators.scala#L231
|
||
expressionValueEvaluator | ||
) | ||
) { (suffix, arr) => | ||
EvaluatedValue(WomArray(arr.value.map(v => WomString(v.valueString + suffix.value))), Seq.empty).validNel | ||
Check warning on line 235 in wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluators.scala Codecov / codecov/patchwdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluators.scala#L234-L235
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the part that actually does the suffix-ing |
||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
draft-3
is not supposed to supportsuffix
, right?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, but all the other post-1.0 engine functions (ex.
min
,max
,sep
) have their case classes in here so I followed that convention. There aren't equivalent files in the other language version packages, though I can make that change if desirable.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like those functions may have been put in there before a next language version was declared and/or before we had our current organization scheme. I do think that separating them out (both current and the new addition) would be optimal, though maybe that's a separate ticket.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Discussed off-github and decided to leave this as-is.