Skip to content

Commit

Permalink
WX-964 suffix() (#7363)
Browse files Browse the repository at this point in the history
  • Loading branch information
jgainerdewar authored Feb 9, 2024
1 parent 13236ff commit a39eabe
Show file tree
Hide file tree
Showing 33 changed files with 234 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -54,4 +54,8 @@ metadata {
"outputs.biscayne_new_engine_functions.bigIntFloatComparison": 10.0
"outputs.biscayne_new_engine_functions.minMaxIntFloatComposition": 1.0
"outputs.biscayne_new_engine_functions.maxIntVsMaxFloat": 1.79769313E+308

"outputs.biscayne_new_engine_functions.with_suffixes.0": "aaaS"
"outputs.biscayne_new_engine_functions.with_suffixes.1": "bbbS"
"outputs.biscayne_new_engine_functions.with_suffixes.2": "cccS"
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ workflow biscayne_new_engine_functions {

meta {
description: "This test makes sure that these functions work in a real workflow"
functions_under_test: [ "keys", "as_map", "as_pairs", "collect_by_key" ]
functions_under_test: [ "keys", "as_map", "as_pairs", "collect_by_key", "suffix" ]
}

Map[String, Int] x_map_in = {"a": 1, "b": 2, "c": 3}
Expand All @@ -15,6 +15,8 @@ workflow biscayne_new_engine_functions {

Array[Pair[String,Int]] z_pairs_in = [("a", 1), ("b", 2), ("a", 3)]

Array[String] some_strings = ["aaa", "bbb", "ccc"]

Int smallestInt = 1
Float smallFloat = 2.718
Float bigFloat = 3.141
Expand Down Expand Up @@ -48,6 +50,10 @@ workflow biscayne_new_engine_functions {
Float bigIntFloatComparison = max(bigFloat, biggestInt) # 10.0
Float minMaxIntFloatComposition = min(max(biggestInt, smallFloat), smallestInt) # 1.0
Float maxIntVsMaxFloat = max(maxInt, maxFloat)

# suffix():
# =================================================
Array[String] with_suffixes = suffix("S", some_strings)
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,12 @@ object ExpressionElement {
override def arg1: ExpressionElement = prefix
override def arg2: ExpressionElement = array
}

final case class Suffix(suffix: ExpressionElement, array: ExpressionElement) extends TwoParamFunctionCallElement {
override def arg1: ExpressionElement = suffix
override def arg2: ExpressionElement = array
}

final case class Min(arg1: ExpressionElement, arg2: ExpressionElement) extends TwoParamFunctionCallElement
final case class Max(arg1: ExpressionElement, arg2: ExpressionElement) extends TwoParamFunctionCallElement
final case class Sep(arg1: ExpressionElement, arg2: ExpressionElement) extends TwoParamFunctionCallElement
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package wdl.transforms.biscayne.ast2wdlom
import cats.syntax.validated._
import common.validation.ErrorOr.ErrorOr
import wdl.model.draft3.elements.ExpressionElement
import wdl.model.draft3.elements.ExpressionElement.{AsMap, AsPairs, CollectByKey, Keys, Max, Min, Sep}
import wdl.model.draft3.elements.ExpressionElement.{AsMap, AsPairs, CollectByKey, Keys, Max, Min, Sep, Suffix}
import wdl.transforms.base.ast2wdlom.AstNodeToExpressionElement

object AstToNewExpressionElements {
Expand All @@ -15,6 +15,7 @@ object AstToNewExpressionElements {
"min" -> AstNodeToExpressionElement.validateTwoParamEngineFunction(Min, "min"),
"max" -> AstNodeToExpressionElement.validateTwoParamEngineFunction(Max, "max"),
"sep" -> AstNodeToExpressionElement.validateTwoParamEngineFunction(Sep, "sep"),
"suffix" -> AstNodeToExpressionElement.validateTwoParamEngineFunction(Suffix, "suffix"),
"read_object" -> (_ =>
"read_object is no longer available in this WDL version. Consider using read_json instead".invalidNel
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,14 @@ object BiscayneExpressionValueConsumers {
expressionValueConsumer.expressionConsumedValueHooks(a.arg2)(expressionValueConsumer)
}

implicit val suffixExpressionValueConsumer: ExpressionValueConsumer[Suffix] = new ExpressionValueConsumer[Suffix] {
override def expressionConsumedValueHooks(a: Suffix)(implicit
expressionValueConsumer: ExpressionValueConsumer[ExpressionElement]
): Set[UnlinkedConsumedValueHook] =
expressionValueConsumer.expressionConsumedValueHooks(a.arg1)(expressionValueConsumer) ++
expressionValueConsumer.expressionConsumedValueHooks(a.arg2)(expressionValueConsumer)
}

implicit val noneLiteralExpressionValueConsumer: ExpressionValueConsumer[NoneLiteralElement.type] =
new ExpressionValueConsumer[NoneLiteralElement.type] {
override def expressionConsumedValueHooks(a: NoneLiteralElement.type)(implicit
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ package object consumed {
case a: Length => a.expressionConsumedValueHooks(expressionValueConsumer)
case a: Flatten => a.expressionConsumedValueHooks(expressionValueConsumer)
case a: Prefix => a.expressionConsumedValueHooks(expressionValueConsumer)
case a: Suffix => a.expressionConsumedValueHooks(expressionValueConsumer)
case a: SelectFirst => a.expressionConsumedValueHooks(expressionValueConsumer)
case a: SelectAll => a.expressionConsumedValueHooks(expressionValueConsumer)
case a: Defined => a.expressionConsumedValueHooks(expressionValueConsumer)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package wdl.transforms.biscayne.linking.expression.files

import wdl.model.draft3.elements.ExpressionElement.{AsMap, AsPairs, CollectByKey, Keys, Max, Min, Sep}
import wdl.model.draft3.elements.ExpressionElement.{AsMap, AsPairs, CollectByKey, Keys, Max, Min, Sep, Suffix}
import wdl.model.draft3.graph.expression.FileEvaluator
import wdl.transforms.base.linking.expression.files.EngineFunctionEvaluators
import wdl.transforms.base.linking.expression.files.EngineFunctionEvaluators.twoParameterFunctionPassthroughFileEvaluator
Expand All @@ -16,6 +16,7 @@ object BiscayneFileEvaluators {
EngineFunctionEvaluators.singleParameterPassthroughFileEvaluator

implicit val sepFunctionEvaluator: FileEvaluator[Sep] = twoParameterFunctionPassthroughFileEvaluator[Sep]
implicit val suffixFunctionEvaluator: FileEvaluator[Suffix] = twoParameterFunctionPassthroughFileEvaluator[Suffix]

implicit val minFunctionEvaluator: FileEvaluator[Min] = twoParameterFunctionPassthroughFileEvaluator[Min]
implicit val maxFunctionEvaluator: FileEvaluator[Max] = twoParameterFunctionPassthroughFileEvaluator[Max]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ package object files {
case a: Flatten =>
a.predictFilesNeededToEvaluate(inputs, ioFunctionSet, coerceTo)(fileEvaluator, valueEvaluator)
case a: Prefix => a.predictFilesNeededToEvaluate(inputs, ioFunctionSet, coerceTo)(fileEvaluator, valueEvaluator)
case a: Suffix => a.predictFilesNeededToEvaluate(inputs, ioFunctionSet, coerceTo)(fileEvaluator, valueEvaluator)
case a: SelectFirst =>
a.predictFilesNeededToEvaluate(inputs, ioFunctionSet, coerceTo)(fileEvaluator, valueEvaluator)
case a: SelectAll =>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package wdl.transforms.biscayne.linking.expression.types

import cats.implicits.catsSyntaxTuple2Semigroupal
import cats.syntax.validated._
import common.validation.ErrorOr._
import wdl.model.draft3.elements.ExpressionElement
Expand Down Expand Up @@ -100,4 +101,13 @@ object BiscayneTypeEvaluators {

}
}

implicit val suffixFunctionEvaluator: TypeEvaluator[Suffix] = new TypeEvaluator[Suffix] {
override def evaluateType(a: Suffix, linkedValues: Map[UnlinkedConsumedValueHook, GeneratedValueHandle])(implicit
expressionTypeEvaluator: TypeEvaluator[ExpressionElement]
): ErrorOr[WomType] =
(validateParamType(a.suffix, linkedValues, WomStringType),
validateParamType(a.array, linkedValues, WomArrayType(WomStringType))
) mapN { (_, _) => WomArrayType(WomStringType) }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ package object types {
case a: Length => a.evaluateType(linkedValues)(typeEvaluator)
case a: Flatten => a.evaluateType(linkedValues)(typeEvaluator)
case a: Prefix => a.evaluateType(linkedValues)(typeEvaluator)
case a: Suffix => a.evaluateType(linkedValues)(typeEvaluator)
case a: SelectFirst => a.evaluateType(linkedValues)(typeEvaluator)
case a: SelectAll => a.evaluateType(linkedValues)(typeEvaluator)
case a: Defined => a.evaluateType(linkedValues)(typeEvaluator)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -217,4 +217,22 @@ object BiscayneValueEvaluators {
EvaluatedValue(WomString(arr.value.map(v => v.valueString).mkString(sepvalue.value)), Seq.empty).validNel
}
}

implicit val suffixFunctionEvaluator: ValueEvaluator[Suffix] = new ValueEvaluator[Suffix] {
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)(
expressionValueEvaluator
),
expressionValueEvaluator.evaluateValue(a.arg2, inputs, ioFunctionSet, forCommandInstantiationOptions)(
expressionValueEvaluator
)
) { (suffix, arr) =>
EvaluatedValue(WomArray(arr.value.map(v => WomString(v.valueString + suffix.value))), Seq.empty).validNel
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,8 @@ package object values {
a.evaluateValue(inputs, ioFunctionSet, forCommandInstantiationOptions)(expressionValueEvaluator)
case a: Prefix =>
a.evaluateValue(inputs, ioFunctionSet, forCommandInstantiationOptions)(expressionValueEvaluator)
case a: Suffix =>
a.evaluateValue(inputs, ioFunctionSet, forCommandInstantiationOptions)(expressionValueEvaluator)
case a: SelectFirst =>
a.evaluateValue(inputs, ioFunctionSet, forCommandInstantiationOptions)(expressionValueEvaluator)
case a: SelectAll =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,4 +97,10 @@ class Ast2WdlomSpec extends AnyFlatSpec with CromwellTimeoutSpec with Matchers {
val expr = fromString[ExpressionElement](str, parser.parse_e)
expr shouldBeValid NoneLiteralElement
}

it should "parse the new suffix function" in {
val str = "suffix(some_str, some_arr)"
val expr = fromString[ExpressionElement](str, parser.parse_e)
expr shouldBeValid (Suffix(IdentifierLookup("some_str"), IdentifierLookup("some_arr")))
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -57,4 +57,22 @@ class BiscayneExpressionValueConsumersSpec extends AnyFlatSpec with CromwellTime
e.expressionConsumedValueHooks should be(Set(UnlinkedIdentifierHook("my_separator"), UnlinkedIdentifierHook("c")))
}
}

it should "discover the variable lookups within a suffix() call" in {
val str = """ suffix(my_suffix, ["a", "b", c]) """
val expr = fromString[ExpressionElement](str, parser.parse_e)

expr.shouldBeValidPF { case e =>
e.expressionConsumedValueHooks should be(Set(UnlinkedIdentifierHook("my_suffix"), UnlinkedIdentifierHook("c")))
}
}

it should "discover an array variable lookup within a suffix() call" in {
val str = """ suffix("SFX", my_array) """
val expr = fromString[ExpressionElement](str, parser.parse_e)

expr.shouldBeValidPF { case e =>
e.expressionConsumedValueHooks should be(Set(UnlinkedIdentifierHook("my_array")))
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -45,4 +45,15 @@ class BiscayneFileEvaluatorSpec extends AnyFlatSpec with CromwellTimeoutSpec wit
)
}
}

it should "discover the file which would be required to evaluate a suffix() function" in {
val str = """ suffix(' # what a line', read_lines("foo.txt")) """
val expr = fromString[ExpressionElement](str, parser.parse_e)

expr.shouldBeValidPF { case e =>
e.predictFilesNeededToEvaluate(Map.empty, NoIoFunctionSet, WomStringType) shouldBeValid Set(
WomSingleFile("foo.txt")
)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -56,4 +56,12 @@ class BiscayneTypeEvaluatorSpec extends AnyFlatSpec with CromwellTimeoutSpec wit
}
}

it should "evaluate the type of a suffix() function as Array[String]" in {
val str = """ suffix('S', ["a", "b", "c"]) """
val expr = fromString[ExpressionElement](str, parser.parse_e)

expr.shouldBeValidPF { case e =>
e.evaluateType(Map.empty) shouldBeValid WomArrayType(WomStringType)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -200,4 +200,21 @@ class BiscayneValueEvaluatorSpec extends AnyFlatSpec with CromwellTimeoutSpec wi
e.evaluateValue(Map.empty, NoIoFunctionSet, None) shouldBeValid EvaluatedValue(expectedString, Seq.empty)
}
}

it should "evaluate a suffix expression correctly" in {
val str = """ suffix("S", ["a", "b", "c"]) """
val expr = fromString[ExpressionElement](str, parser.parse_e)

val expectedArray: WomArray = WomArray(
Seq(
WomString("aS"),
WomString("bS"),
WomString("cS")
)
)

expr.shouldBeValidPF { case e =>
e.evaluateValue(Map.empty, NoIoFunctionSet, None) shouldBeValid EvaluatedValue(expectedArray, Seq.empty)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package wdl.transforms.cascades.ast2wdlom
import cats.syntax.validated._
import common.validation.ErrorOr.ErrorOr
import wdl.model.draft3.elements.ExpressionElement
import wdl.model.draft3.elements.ExpressionElement.{AsMap, AsPairs, CollectByKey, Keys, Max, Min, Sep}
import wdl.model.draft3.elements.ExpressionElement.{AsMap, AsPairs, CollectByKey, Keys, Max, Min, Sep, Suffix}
import wdl.transforms.base.ast2wdlom.AstNodeToExpressionElement

object AstToNewExpressionElements {
Expand All @@ -15,6 +15,7 @@ object AstToNewExpressionElements {
"min" -> AstNodeToExpressionElement.validateTwoParamEngineFunction(Min, "min"),
"max" -> AstNodeToExpressionElement.validateTwoParamEngineFunction(Max, "max"),
"sep" -> AstNodeToExpressionElement.validateTwoParamEngineFunction(Sep, "sep"),
"suffix" -> AstNodeToExpressionElement.validateTwoParamEngineFunction(Suffix, "suffix"),
"read_object" -> (_ =>
"read_object is no longer available in this WDL version. Consider using read_json instead".invalidNel
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,14 @@ object cascadesExpressionValueConsumers {
expressionValueConsumer.expressionConsumedValueHooks(a.arg2)(expressionValueConsumer)
}

implicit val suffixExpressionValueConsumer: ExpressionValueConsumer[Suffix] = new ExpressionValueConsumer[Suffix] {
override def expressionConsumedValueHooks(a: Suffix)(implicit
expressionValueConsumer: ExpressionValueConsumer[ExpressionElement]
): Set[UnlinkedConsumedValueHook] =
expressionValueConsumer.expressionConsumedValueHooks(a.arg1)(expressionValueConsumer) ++
expressionValueConsumer.expressionConsumedValueHooks(a.arg2)(expressionValueConsumer)
}

implicit val noneLiteralExpressionValueConsumer: ExpressionValueConsumer[NoneLiteralElement.type] =
new ExpressionValueConsumer[NoneLiteralElement.type] {
override def expressionConsumedValueHooks(a: NoneLiteralElement.type)(implicit
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ package object consumed {
case a: Length => a.expressionConsumedValueHooks(expressionValueConsumer)
case a: Flatten => a.expressionConsumedValueHooks(expressionValueConsumer)
case a: Prefix => a.expressionConsumedValueHooks(expressionValueConsumer)
case a: Suffix => a.expressionConsumedValueHooks(expressionValueConsumer)
case a: SelectFirst => a.expressionConsumedValueHooks(expressionValueConsumer)
case a: SelectAll => a.expressionConsumedValueHooks(expressionValueConsumer)
case a: Defined => a.expressionConsumedValueHooks(expressionValueConsumer)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package wdl.transforms.cascades.linking.expression.files

import wdl.model.draft3.elements.ExpressionElement.{AsMap, AsPairs, CollectByKey, Keys, Max, Min, Sep}
import wdl.model.draft3.elements.ExpressionElement.{AsMap, AsPairs, CollectByKey, Keys, Max, Min, Sep, Suffix}
import wdl.model.draft3.graph.expression.FileEvaluator
import wdl.transforms.base.linking.expression.files.EngineFunctionEvaluators
import wdl.transforms.base.linking.expression.files.EngineFunctionEvaluators.twoParameterFunctionPassthroughFileEvaluator
Expand All @@ -16,6 +16,7 @@ object cascadesFileEvaluators {
EngineFunctionEvaluators.singleParameterPassthroughFileEvaluator

implicit val sepFunctionEvaluator: FileEvaluator[Sep] = twoParameterFunctionPassthroughFileEvaluator[Sep]
implicit val suffixFunctionEvaluator: FileEvaluator[Suffix] = twoParameterFunctionPassthroughFileEvaluator[Suffix]

implicit val minFunctionEvaluator: FileEvaluator[Min] = twoParameterFunctionPassthroughFileEvaluator[Min]
implicit val maxFunctionEvaluator: FileEvaluator[Max] = twoParameterFunctionPassthroughFileEvaluator[Max]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ package object files {
case a: Flatten =>
a.predictFilesNeededToEvaluate(inputs, ioFunctionSet, coerceTo)(fileEvaluator, valueEvaluator)
case a: Prefix => a.predictFilesNeededToEvaluate(inputs, ioFunctionSet, coerceTo)(fileEvaluator, valueEvaluator)
case a: Suffix => a.predictFilesNeededToEvaluate(inputs, ioFunctionSet, coerceTo)(fileEvaluator, valueEvaluator)
case a: SelectFirst =>
a.predictFilesNeededToEvaluate(inputs, ioFunctionSet, coerceTo)(fileEvaluator, valueEvaluator)
case a: SelectAll =>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package wdl.transforms.biscayne.linking.expression.types

import cats.implicits.catsSyntaxTuple2Semigroupal
import cats.syntax.validated._
import common.validation.ErrorOr._
import wdl.model.draft3.elements.ExpressionElement
Expand Down Expand Up @@ -100,4 +101,13 @@ object cascadesTypeEvaluators {

}
}

implicit val suffixFunctionEvaluator: TypeEvaluator[Suffix] = new TypeEvaluator[Suffix] {
override def evaluateType(a: Suffix, linkedValues: Map[UnlinkedConsumedValueHook, GeneratedValueHandle])(implicit
expressionTypeEvaluator: TypeEvaluator[ExpressionElement]
): ErrorOr[WomType] =
(validateParamType(a.suffix, linkedValues, WomStringType),
validateParamType(a.array, linkedValues, WomArrayType(WomStringType))
) mapN { (_, _) => WomArrayType(WomStringType) }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ package object types {
case a: Length => a.evaluateType(linkedValues)(typeEvaluator)
case a: Flatten => a.evaluateType(linkedValues)(typeEvaluator)
case a: Prefix => a.evaluateType(linkedValues)(typeEvaluator)
case a: Suffix => a.evaluateType(linkedValues)(typeEvaluator)
case a: SelectFirst => a.evaluateType(linkedValues)(typeEvaluator)
case a: SelectAll => a.evaluateType(linkedValues)(typeEvaluator)
case a: Defined => a.evaluateType(linkedValues)(typeEvaluator)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -217,4 +217,22 @@ object cascadesValueEvaluators {
EvaluatedValue(WomString(arr.value.map(v => v.valueString).mkString(sepvalue.value)), Seq.empty).validNel
}
}

implicit val suffixFunctionEvaluator: ValueEvaluator[Suffix] = new ValueEvaluator[Suffix] {
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)(
expressionValueEvaluator
),
expressionValueEvaluator.evaluateValue(a.arg2, inputs, ioFunctionSet, forCommandInstantiationOptions)(
expressionValueEvaluator
)
) { (suffix, arr) =>
EvaluatedValue(WomArray(arr.value.map(v => WomString(v.valueString + suffix.value))), Seq.empty).validNel
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,8 @@ package object values {
a.evaluateValue(inputs, ioFunctionSet, forCommandInstantiationOptions)(expressionValueEvaluator)
case a: Prefix =>
a.evaluateValue(inputs, ioFunctionSet, forCommandInstantiationOptions)(expressionValueEvaluator)
case a: Suffix =>
a.evaluateValue(inputs, ioFunctionSet, forCommandInstantiationOptions)(expressionValueEvaluator)
case a: SelectFirst =>
a.evaluateValue(inputs, ioFunctionSet, forCommandInstantiationOptions)(expressionValueEvaluator)
case a: SelectAll =>
Expand Down
Loading

0 comments on commit a39eabe

Please sign in to comment.