From c8a7d975f7e44bc14059a8abdd41c016443f0d3b Mon Sep 17 00:00:00 2001 From: Tom Wiseman Date: Wed, 14 Feb 2024 14:42:07 -0500 Subject: [PATCH 01/20] unzip --- .../draft3/elements/ExpressionElement.scala | 1 + .../values/CascadesValueEvaluators.scala | 23 ++++++++++++++++++- .../base/wdlom2wdl/WdlWriterImpl.scala | 1 + 3 files changed, 24 insertions(+), 1 deletion(-) diff --git a/wdl/model/draft3/src/main/scala/wdl/model/draft3/elements/ExpressionElement.scala b/wdl/model/draft3/src/main/scala/wdl/model/draft3/elements/ExpressionElement.scala index a2fc6202193..f655feaeba4 100644 --- a/wdl/model/draft3/src/main/scala/wdl/model/draft3/elements/ExpressionElement.scala +++ b/wdl/model/draft3/src/main/scala/wdl/model/draft3/elements/ExpressionElement.scala @@ -136,6 +136,7 @@ object ExpressionElement { final case class Ceil(param: ExpressionElement) extends OneParamFunctionCallElement final case class Round(param: ExpressionElement) extends OneParamFunctionCallElement final case class Glob(param: ExpressionElement) extends OneParamFunctionCallElement + final case class Unzip(param: ExpressionElement) extends OneParamFunctionCallElement // 1- or 2-param functions: sealed trait OneOrTwoParamFunctionCallElement extends FunctionCallElement { diff --git a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluators.scala b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluators.scala index 788f114e181..ac9764d1a4c 100644 --- a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluators.scala +++ b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluators.scala @@ -217,4 +217,25 @@ object cascadesValueEvaluators { EvaluatedValue(WomString(arr.value.map(v => v.valueString).mkString(sepvalue.value)), Seq.empty).validNel } } -} + + implicit val unzipFunctionEvaluator: ValueEvaluator[Unzip] = new ValueEvaluator[Unzip] { + override def evaluateValue(a: Unzip, + inputs: Map[String, WomValue], + ioFunctionSet: IoFunctionSet, + forCommandInstantiationOptions: Option[ForCommandInstantiationOptions] + )(implicit expressionValueEvaluator: ValueEvaluator[ExpressionElement]): ErrorOr[EvaluatedValue[WomPair]] = { + + processValidatedSingleValue[WomArray, WomPair](a.param: WomArray) + processTwoValidatedValues[WomArray, WomArray, WomArray]( + a.param.evaluateValue(inputs, ioFunctionSet, forCommandInstantiationOptions), + a.arg2.evaluateValue(inputs, ioFunctionSet, forCommandInstantiationOptions) + ) { (arr1, arr2) => + val pairs = for { + a <- arr1.value + b <- arr2.value + } yield WomPair(a, b) + EvaluatedValue(WomArray(WomArrayType(WomPairType(arr1.arrayType.memberType, arr2.arrayType.memberType)), pairs), + Seq.empty + ).validNel + } + } diff --git a/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/wdlom2wdl/WdlWriterImpl.scala b/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/wdlom2wdl/WdlWriterImpl.scala index 70cc56b5a65..b4fd0165889 100644 --- a/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/wdlom2wdl/WdlWriterImpl.scala +++ b/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/wdlom2wdl/WdlWriterImpl.scala @@ -439,6 +439,7 @@ object WdlWriterImpl { case _: AsMap => "as_map" case _: AsPairs => "as_pairs" case _: CollectByKey => "collect_by_key" + case _: Unzip => "unzip" } s"$fn(${a.param.toWdlV1})" From 43c61783e48922cdb1708aefd9739a31eb5ed867 Mon Sep 17 00:00:00 2001 From: Tom Wiseman Date: Wed, 14 Feb 2024 16:05:49 -0500 Subject: [PATCH 02/20] v1 --- .../values/CascadesValueEvaluators.scala | 35 +++++++++++-------- 1 file changed, 21 insertions(+), 14 deletions(-) diff --git a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluators.scala b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluators.scala index ad7ef2ab271..0f7e71994d4 100644 --- a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluators.scala +++ b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluators.scala @@ -235,26 +235,33 @@ object cascadesValueEvaluators { EvaluatedValue(WomArray(arr.value.map(v => WomString(v.valueString + suffix.value))), Seq.empty).validNel } } - + // Pair[Array[X], Array[Y]] unzip(Array[Pair[X, Y]]) + // Creates a Pair of Arrays, the first containing the elements from the left members of an Array of Pairs, and the second containing the right members. + // This is the inverse of the zip function. + // @params : Array[Pair[X,Y]] + // @returns : Pair[Array[X], Array[Y]] implicit val unzipFunctionEvaluator: ValueEvaluator[Unzip] = new ValueEvaluator[Unzip] { override def evaluateValue(a: Unzip, inputs: Map[String, WomValue], ioFunctionSet: IoFunctionSet, forCommandInstantiationOptions: Option[ForCommandInstantiationOptions] )(implicit expressionValueEvaluator: ValueEvaluator[ExpressionElement]): ErrorOr[EvaluatedValue[WomPair]] = { - - processValidatedSingleValue[WomArray, WomPair](a.param: WomArray) - processTwoValidatedValues[WomArray, WomArray, WomArray]( - a.param.evaluateValue(inputs, ioFunctionSet, forCommandInstantiationOptions), - a.arg2.evaluateValue(inputs, ioFunctionSet, forCommandInstantiationOptions) - ) { (arr1, arr2) => - val pairs = for { - a <- arr1.value - b <- arr2.value - } yield WomPair(a, b) - EvaluatedValue(WomArray(WomArrayType(WomPairType(arr1.arrayType.memberType, arr2.arrayType.memberType)), pairs), - Seq.empty - ).validNel + processValidatedSingleValue[WomArray, WomPair]( + expressionValueEvaluator.evaluateValue(a.param, inputs, ioFunctionSet, forCommandInstantiationOptions)( + expressionValueEvaluator + ) + ) { + case WomArray(WomArrayType(WomPairType(leftType, rightType)), values) => + val zippedPairs: List[WomPair] = values.toList map { case pair: WomPair => + WomPair(pair.left, pair.right) + } + val left: WomArray = WomArray(zippedPairs.map(pair => pair.left)) + val right: WomArray = WomArray(zippedPairs.map(pair => pair.right)) + val unzippedPairs: WomPair = WomPair(left, right) + EvaluatedValue(unzippedPairs, Seq.empty).validNel + case other => + s"Invalid call of 'unzip' on parameter of type '${other.womType.stableName}' (expected Array[Pair[X, Y]])".invalidNel } } + } } From f3cfd77327e3ea81bad7a585390da4bb1d06e7b0 Mon Sep 17 00:00:00 2001 From: Tom Wiseman Date: Thu, 15 Feb 2024 15:59:30 -0500 Subject: [PATCH 03/20] biscayne mapping --- .../biscayne_new_engine_functions.test | 6 ++++++ .../biscayne_new_engine_functions.wdl | 16 +++++++++++++++- .../ast2wdlom/AstToNewExpressionElements.scala | 3 ++- .../BiscayneExpressionValueConsumers.scala | 6 ++++++ .../linking/expression/consumed/consumed.scala | 1 + .../files/BiscayneFileEvaluators.scala | 3 ++- .../linking/expression/files/files.scala | 1 + .../types/BiscayneTypeEvaluators.scala | 8 ++++++++ .../linking/expression/types/types.scala | 1 + .../linking/expression/values/values.scala | 1 + .../values/CascadesValueEvaluators.scala | 9 ++++----- 11 files changed, 47 insertions(+), 8 deletions(-) diff --git a/centaur/src/main/resources/standardTestCases/biscayne_new_engine_functions.test b/centaur/src/main/resources/standardTestCases/biscayne_new_engine_functions.test index 6998d4af7e7..1d06c7a21b6 100644 --- a/centaur/src/main/resources/standardTestCases/biscayne_new_engine_functions.test +++ b/centaur/src/main/resources/standardTestCases/biscayne_new_engine_functions.test @@ -58,4 +58,10 @@ metadata { "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" + + "outputs.biscayne_new_engine_functions.unzipped_a": ([],[]) + "outputs.biscayne_new_engine_functions.unzipped_b": (["A"],["a"]) + "outputs.biscayne_new_engine_functions.unzipped_c": (["A","B"],["a","b"]) + "outputs.biscayne_new_engine_functions.unzipped_d": (["A","B","C"],["a","b","c"]) + "outputs.biscayne_new_engine_functions.unzipped_e": (["one","two","three"],[1.0,2.0,3.0]) } diff --git a/centaur/src/main/resources/standardTestCases/wdl_biscayne/biscayne_new_engine_functions/biscayne_new_engine_functions.wdl b/centaur/src/main/resources/standardTestCases/wdl_biscayne/biscayne_new_engine_functions/biscayne_new_engine_functions.wdl index f7b3d0de7fd..eb10edd6f83 100644 --- a/centaur/src/main/resources/standardTestCases/wdl_biscayne/biscayne_new_engine_functions/biscayne_new_engine_functions.wdl +++ b/centaur/src/main/resources/standardTestCases/wdl_biscayne/biscayne_new_engine_functions/biscayne_new_engine_functions.wdl @@ -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", "suffix" ] + functions_under_test: [ "keys", "as_map", "as_pairs", "collect_by_key", "suffix", "unzip" ] } Map[String, Int] x_map_in = {"a": 1, "b": 2, "c": 3} @@ -26,6 +26,12 @@ workflow biscayne_new_engine_functions { # max float... near enough: Float maxFloat = 179769313000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000.0 + Array[Pair[String,String]] zipped_a = [] + Array[Pair[String,String]] zipped_b = [("A","a")] + Array[Pair[String,String]] zipped_c = [("A","a"),("B","b")] + Array[Pair[String,String]] zipped_d = [("A","a"),("B","b"),("C","c")] + Array[Pair[String,Float]] zipped_e = [("one",1.0),("two",2.0),("three",3.0)] + output { # keys(), as_map(), as_pairs(), collect_by_key(): @@ -54,6 +60,14 @@ workflow biscayne_new_engine_functions { # suffix(): # ================================================= Array[String] with_suffixes = suffix("S", some_strings) + + # unzip(): + # ================================================= + Pair[Array[String], Array[String]] unzipped_a = unzip(zipped_a) + Pair[Array[String], Array[String]] unzipped_b = unzip(zipped_b) + Pair[Array[String], Array[String]] unzipped_c = unzip(zipped_c) + Pair[Array[String], Array[String]] unzipped_d = unzip(zipped_d) + Pair[Array[String], Array[Float]] unzipped_e = unzip(zipped_e) } } diff --git a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/ast2wdlom/AstToNewExpressionElements.scala b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/ast2wdlom/AstToNewExpressionElements.scala index 5c0c01859c2..56e452953b8 100644 --- a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/ast2wdlom/AstToNewExpressionElements.scala +++ b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/ast2wdlom/AstToNewExpressionElements.scala @@ -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, Suffix} +import wdl.model.draft3.elements.ExpressionElement.{AsMap, AsPairs, CollectByKey, Keys, Max, Min, Sep, Suffix, Unzip} import wdl.transforms.base.ast2wdlom.AstNodeToExpressionElement object AstToNewExpressionElements { @@ -16,6 +16,7 @@ object AstToNewExpressionElements { "max" -> AstNodeToExpressionElement.validateTwoParamEngineFunction(Max, "max"), "sep" -> AstNodeToExpressionElement.validateTwoParamEngineFunction(Sep, "sep"), "suffix" -> AstNodeToExpressionElement.validateTwoParamEngineFunction(Suffix, "suffix"), + "unzip" -> AstNodeToExpressionElement.validateOneParamEngineFunction(Unzip, "unzip"), "read_object" -> (_ => "read_object is no longer available in this WDL version. Consider using read_json instead".invalidNel ), diff --git a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/consumed/BiscayneExpressionValueConsumers.scala b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/consumed/BiscayneExpressionValueConsumers.scala index 3c73f8b55cd..c78bde94a46 100644 --- a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/consumed/BiscayneExpressionValueConsumers.scala +++ b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/consumed/BiscayneExpressionValueConsumers.scala @@ -66,6 +66,12 @@ object BiscayneExpressionValueConsumers { expressionValueConsumer.expressionConsumedValueHooks(a.arg2)(expressionValueConsumer) } + implicit val unzipExpressionValueConsumer: ExpressionValueConsumer[Unzip] = new ExpressionValueConsumer[Suffix] { + override def expressionConsumedValueHooks(a: Unzip)(implicit + expressionValueConsumer: ExpressionValueConsumer[ExpressionElement]): Set[UnlinkedConsumedValueHook] = + expressionValueConsumer.expressionConsumedValueHooks(a.param)(expressionValueConsumer) + } + implicit val noneLiteralExpressionValueConsumer: ExpressionValueConsumer[NoneLiteralElement.type] = new ExpressionValueConsumer[NoneLiteralElement.type] { override def expressionConsumedValueHooks(a: NoneLiteralElement.type)(implicit diff --git a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/consumed/consumed.scala b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/consumed/consumed.scala index e1121e1d549..0d5ea83f7e6 100644 --- a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/consumed/consumed.scala +++ b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/consumed/consumed.scala @@ -104,6 +104,7 @@ package object consumed { case a: AsPairs => a.expressionConsumedValueHooks(expressionValueConsumer) case a: CollectByKey => a.expressionConsumedValueHooks(expressionValueConsumer) case a: Sep => sepExpressionValueConsumer.expressionConsumedValueHooks(a)(expressionValueConsumer) + case a: Unzip => a.expressionConsumedValueHooks(expressionValueConsumer) case a: Min => a.expressionConsumedValueHooks(expressionValueConsumer) case a: Max => a.expressionConsumedValueHooks(expressionValueConsumer) diff --git a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/files/BiscayneFileEvaluators.scala b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/files/BiscayneFileEvaluators.scala index 6a79d2b3948..accb2a9435b 100644 --- a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/files/BiscayneFileEvaluators.scala +++ b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/files/BiscayneFileEvaluators.scala @@ -1,6 +1,6 @@ package wdl.transforms.biscayne.linking.expression.files -import wdl.model.draft3.elements.ExpressionElement.{AsMap, AsPairs, CollectByKey, Keys, Max, Min, Sep, Suffix} +import wdl.model.draft3.elements.ExpressionElement.{AsMap, AsPairs, CollectByKey, Keys, Max, Min, Sep, Suffix, Unzip} import wdl.model.draft3.graph.expression.FileEvaluator import wdl.transforms.base.linking.expression.files.EngineFunctionEvaluators import wdl.transforms.base.linking.expression.files.EngineFunctionEvaluators.twoParameterFunctionPassthroughFileEvaluator @@ -21,4 +21,5 @@ object BiscayneFileEvaluators { implicit val minFunctionEvaluator: FileEvaluator[Min] = twoParameterFunctionPassthroughFileEvaluator[Min] implicit val maxFunctionEvaluator: FileEvaluator[Max] = twoParameterFunctionPassthroughFileEvaluator[Max] + implicit val unzipFunctionEvaluator: FileEvaluator[Unzip] = EngineFunctionEvaluators.singleParameterPassthroughFileEvaluator } diff --git a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/files/files.scala b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/files/files.scala index 9dd95e9a0ec..e4dc188563d 100644 --- a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/files/files.scala +++ b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/files/files.scala @@ -153,6 +153,7 @@ package object files { case a: Zip => a.predictFilesNeededToEvaluate(inputs, ioFunctionSet, coerceTo)(fileEvaluator, valueEvaluator) case a: Cross => a.predictFilesNeededToEvaluate(inputs, ioFunctionSet, coerceTo)(fileEvaluator, valueEvaluator) + case a: Unzip => a.predictFilesNeededToEvaluate(inputs, ioFunctionSet, coerceTo)(fileEvaluator, valueEvaluator) case a: Sub => a.predictFilesNeededToEvaluate(inputs, ioFunctionSet, coerceTo)(fileEvaluator, valueEvaluator) diff --git a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/types/BiscayneTypeEvaluators.scala b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/types/BiscayneTypeEvaluators.scala index 1ff5bf71ba4..d5ccaac30aa 100644 --- a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/types/BiscayneTypeEvaluators.scala +++ b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/types/BiscayneTypeEvaluators.scala @@ -110,4 +110,12 @@ object BiscayneTypeEvaluators { validateParamType(a.array, linkedValues, WomArrayType(WomStringType)) ) mapN { (_, _) => WomArrayType(WomStringType) } } + + implicit val unzipFunctionEvaluator: TypeEvaluator[Unzip] = new TypeEvaluator[Unzip]{ + override def evaluateType(a: Unzip, linkedValues: Map[UnlinkedConsumedValueHook, GeneratedValueHandle])(implicit expressionTypeEvaluator: TypeEvaluator[ExpressionElement]) : ErrorOr[WomType] = + validateParamType(a.param, linkedValues, WomArrayType(WomPairType(WomAnyType, WomAnyType))) flatMap { + case WomArrayType(WomPairType(x,y)) => WomPairType(WomArrayType(x), WomArrayType(y)).validNel + case other => s"Cannot invoke 'unzip' on type '${other.stableName}'. Expected an array of pairs".invalidNel + } + } } diff --git a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/types/types.scala b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/types/types.scala index dec2deeb24f..2f1eddbe670 100644 --- a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/types/types.scala +++ b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/types/types.scala @@ -97,6 +97,7 @@ package object types { case a: Zip => a.evaluateType(linkedValues)(typeEvaluator) case a: Cross => a.evaluateType(linkedValues)(typeEvaluator) + case a: Unzip => a.evaluateType(linkedValues)(typeEvaluator) case a: Sub => a.evaluateType(linkedValues)(typeEvaluator) diff --git a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/values/values.scala b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/values/values.scala index 3106ceba428..878456c5038 100644 --- a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/values/values.scala +++ b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/values/values.scala @@ -164,6 +164,7 @@ package object values { case a: Zip => a.evaluateValue(inputs, ioFunctionSet, forCommandInstantiationOptions)(expressionValueEvaluator) case a: Cross => a.evaluateValue(inputs, ioFunctionSet, forCommandInstantiationOptions)(expressionValueEvaluator) + case a: Unzip => a.evaluateValue(inputs, ioFunctionSet, forCommandInstantiationOptions)(expressionValueEvaluator) case a: Sub => a.evaluateValue(inputs, ioFunctionSet, forCommandInstantiationOptions)(expressionValueEvaluator) diff --git a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluators.scala b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluators.scala index 0f7e71994d4..c7726174507 100644 --- a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluators.scala +++ b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluators.scala @@ -252,12 +252,11 @@ object cascadesValueEvaluators { ) ) { case WomArray(WomArrayType(WomPairType(leftType, rightType)), values) => - val zippedPairs: List[WomPair] = values.toList map { case pair: WomPair => - WomPair(pair.left, pair.right) + val zippedPairs: Seq[(WomValue, WomValue)] = values map { case pair: WomPair => + Tuple2(pair.left, pair.right) } - val left: WomArray = WomArray(zippedPairs.map(pair => pair.left)) - val right: WomArray = WomArray(zippedPairs.map(pair => pair.right)) - val unzippedPairs: WomPair = WomPair(left, right) + val (left, right) = zippedPairs.unzip + val unzippedPairs: WomPair = WomPair(WomArray(left), WomArray(right)) EvaluatedValue(unzippedPairs, Seq.empty).validNel case other => s"Invalid call of 'unzip' on parameter of type '${other.womType.stableName}' (expected Array[Pair[X, Y]])".invalidNel From aa931b0b80d983c512e4cf5ad748d805e85a84b5 Mon Sep 17 00:00:00 2001 From: Tom Wiseman Date: Fri, 16 Feb 2024 14:13:08 -0500 Subject: [PATCH 04/20] switch to biscayne for now --- .../values/BiscayneValueEvaluators.scala | 29 +++++++++++++++++++ .../values/CascadesValueEvaluators.scala | 28 ------------------ 2 files changed, 29 insertions(+), 28 deletions(-) diff --git a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluators.scala b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluators.scala index 0c4805f0e68..c835992d3dd 100644 --- a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluators.scala +++ b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluators.scala @@ -235,4 +235,33 @@ object BiscayneValueEvaluators { EvaluatedValue(WomArray(arr.value.map(v => WomString(v.valueString + suffix.value))), Seq.empty).validNel } } + + // Pair[Array[X], Array[Y]] unzip(Array[Pair[X, Y]]) + // Creates a Pair of Arrays, the first containing the elements from the left members of an Array of Pairs, and the second containing the right members. + // This is the inverse of the zip function. + // @params : Array[Pair[X,Y]] + // @returns : Pair[Array[X], Array[Y]] + implicit val unzipFunctionEvaluator: ValueEvaluator[Unzip] = new ValueEvaluator[Unzip] { + override def evaluateValue(a: Unzip, + inputs: Map[String, WomValue], + ioFunctionSet: IoFunctionSet, + forCommandInstantiationOptions: Option[ForCommandInstantiationOptions] + )(implicit expressionValueEvaluator: ValueEvaluator[ExpressionElement]): ErrorOr[EvaluatedValue[WomPair]] = { + processValidatedSingleValue[WomArray, WomPair]( + expressionValueEvaluator.evaluateValue(a.param, inputs, ioFunctionSet, forCommandInstantiationOptions)( + expressionValueEvaluator + ) + ) { + case WomArray(WomArrayType(WomPairType(leftType, rightType)), values) => + val zippedPairs: Seq[(WomValue, WomValue)] = values map { case pair: WomPair => + Tuple2(pair.left, pair.right) + } + val (left, right) = zippedPairs.unzip + val unzippedPairs: WomPair = WomPair(WomArray(left), WomArray(right)) + EvaluatedValue(unzippedPairs, Seq.empty).validNel + case other => + s"Invalid call of 'unzip' on parameter of type '${other.womType.stableName}' (expected Array[Pair[X, Y]])".invalidNel + } + } + } } diff --git a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluators.scala b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluators.scala index c7726174507..db9df85bf06 100644 --- a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluators.scala +++ b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluators.scala @@ -235,32 +235,4 @@ object cascadesValueEvaluators { EvaluatedValue(WomArray(arr.value.map(v => WomString(v.valueString + suffix.value))), Seq.empty).validNel } } - // Pair[Array[X], Array[Y]] unzip(Array[Pair[X, Y]]) - // Creates a Pair of Arrays, the first containing the elements from the left members of an Array of Pairs, and the second containing the right members. - // This is the inverse of the zip function. - // @params : Array[Pair[X,Y]] - // @returns : Pair[Array[X], Array[Y]] - implicit val unzipFunctionEvaluator: ValueEvaluator[Unzip] = new ValueEvaluator[Unzip] { - override def evaluateValue(a: Unzip, - inputs: Map[String, WomValue], - ioFunctionSet: IoFunctionSet, - forCommandInstantiationOptions: Option[ForCommandInstantiationOptions] - )(implicit expressionValueEvaluator: ValueEvaluator[ExpressionElement]): ErrorOr[EvaluatedValue[WomPair]] = { - processValidatedSingleValue[WomArray, WomPair]( - expressionValueEvaluator.evaluateValue(a.param, inputs, ioFunctionSet, forCommandInstantiationOptions)( - expressionValueEvaluator - ) - ) { - case WomArray(WomArrayType(WomPairType(leftType, rightType)), values) => - val zippedPairs: Seq[(WomValue, WomValue)] = values map { case pair: WomPair => - Tuple2(pair.left, pair.right) - } - val (left, right) = zippedPairs.unzip - val unzippedPairs: WomPair = WomPair(WomArray(left), WomArray(right)) - EvaluatedValue(unzippedPairs, Seq.empty).validNel - case other => - s"Invalid call of 'unzip' on parameter of type '${other.womType.stableName}' (expected Array[Pair[X, Y]])".invalidNel - } - } - } } From 5c634f30d552db4cc04ab0867cc5b8bf0aa9d569 Mon Sep 17 00:00:00 2001 From: Tom Wiseman Date: Tue, 20 Feb 2024 15:58:26 -0500 Subject: [PATCH 05/20] test work --- .../BiscayneExpressionValueConsumers.scala | 2 +- .../values/BiscayneValueEvaluators.scala | 3 +- .../transforms/biscayne/Ast2WdlomSpec.scala | 6 ++++ ...BiscayneExpressionValueConsumersSpec.scala | 9 ++++++ .../files/BiscayneFileEvaluatorSpec.scala | 11 ++++++++ .../types/BiscayneTypeEvaluatorSpec.scala | 14 ++++++++++ .../values/BiscayneValueEvaluatorSpec.scala | 28 ++++++++++++++++++- 7 files changed, 70 insertions(+), 3 deletions(-) diff --git a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/consumed/BiscayneExpressionValueConsumers.scala b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/consumed/BiscayneExpressionValueConsumers.scala index c78bde94a46..057af06a6b3 100644 --- a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/consumed/BiscayneExpressionValueConsumers.scala +++ b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/consumed/BiscayneExpressionValueConsumers.scala @@ -66,7 +66,7 @@ object BiscayneExpressionValueConsumers { expressionValueConsumer.expressionConsumedValueHooks(a.arg2)(expressionValueConsumer) } - implicit val unzipExpressionValueConsumer: ExpressionValueConsumer[Unzip] = new ExpressionValueConsumer[Suffix] { + implicit val unzipExpressionValueConsumer: ExpressionValueConsumer[Unzip] = new ExpressionValueConsumer[Unzip] { override def expressionConsumedValueHooks(a: Unzip)(implicit expressionValueConsumer: ExpressionValueConsumer[ExpressionElement]): Set[UnlinkedConsumedValueHook] = expressionValueConsumer.expressionConsumedValueHooks(a.param)(expressionValueConsumer) diff --git a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluators.scala b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluators.scala index c835992d3dd..d96f7481a8c 100644 --- a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluators.scala +++ b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluators.scala @@ -252,7 +252,8 @@ object BiscayneValueEvaluators { expressionValueEvaluator ) ) { - case WomArray(WomArrayType(WomPairType(leftType, rightType)), values) => + case WomArray(WomArrayType(WomAnyType), Seq()) => EvaluatedValue(WomPair(WomArray(WomArrayType(WomAnyType), Seq.empty), WomArray(WomArrayType(WomAnyType), Seq.empty)),Seq.empty).validNel + case WomArray(WomArrayType(WomPairType(_,_)), values) => val zippedPairs: Seq[(WomValue, WomValue)] = values map { case pair: WomPair => Tuple2(pair.left, pair.right) } diff --git a/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/Ast2WdlomSpec.scala b/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/Ast2WdlomSpec.scala index a0953cd8397..3c8abad5203 100644 --- a/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/Ast2WdlomSpec.scala +++ b/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/Ast2WdlomSpec.scala @@ -103,4 +103,10 @@ class Ast2WdlomSpec extends AnyFlatSpec with CromwellTimeoutSpec with Matchers { val expr = fromString[ExpressionElement](str, parser.parse_e) expr shouldBeValid (Suffix(IdentifierLookup("some_str"), IdentifierLookup("some_arr"))) } + + it should "parse the new unzip function" in { + val str = "unzip(some_array_of_pairs)" + val expr = fromString[ExpressionElement](str, parser.parse_e) + expr shouldBeValid (Unzip(IdentifierLookup("some_array_of_pairs"))) + } } diff --git a/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/consumed/BiscayneExpressionValueConsumersSpec.scala b/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/consumed/BiscayneExpressionValueConsumersSpec.scala index 76f4a0338e7..53c63050bef 100644 --- a/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/consumed/BiscayneExpressionValueConsumersSpec.scala +++ b/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/consumed/BiscayneExpressionValueConsumersSpec.scala @@ -75,4 +75,13 @@ class BiscayneExpressionValueConsumersSpec extends AnyFlatSpec with CromwellTime e.expressionConsumedValueHooks should be(Set(UnlinkedIdentifierHook("my_array"))) } } + + it should "discover an array variable lookup within a unzip() call" in { + val str = """ unzip(my_array_of_pairs) """ + val expr = fromString[ExpressionElement](str, parser.parse_e) + + expr.shouldBeValidPF { case e => + e.expressionConsumedValueHooks should be(Set(UnlinkedIdentifierHook("my_array_of_pairs"))) + } + } } diff --git a/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/files/BiscayneFileEvaluatorSpec.scala b/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/files/BiscayneFileEvaluatorSpec.scala index 4a3ed6b63b9..21d92fe9fe3 100644 --- a/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/files/BiscayneFileEvaluatorSpec.scala +++ b/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/files/BiscayneFileEvaluatorSpec.scala @@ -56,4 +56,15 @@ class BiscayneFileEvaluatorSpec extends AnyFlatSpec with CromwellTimeoutSpec wit ) } } + + it should "discover the file which would be required to evaluate a unzip() function" in { + val str = """ unzip(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") + ) + } + } } diff --git a/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/types/BiscayneTypeEvaluatorSpec.scala b/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/types/BiscayneTypeEvaluatorSpec.scala index c8372d36cd1..43ee0931751 100644 --- a/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/types/BiscayneTypeEvaluatorSpec.scala +++ b/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/types/BiscayneTypeEvaluatorSpec.scala @@ -64,4 +64,18 @@ class BiscayneTypeEvaluatorSpec extends AnyFlatSpec with CromwellTimeoutSpec wit e.evaluateType(Map.empty) shouldBeValid WomArrayType(WomStringType) } } + + it should "evaluate the type of a unzip() function as Pair[Array[X], Array[Y]]" in { + val string_and_int = """ unzip([("one", 1),("two", 2),("three", 3)]) """ + val string_and_int_expr = fromString[ExpressionElement](string_and_int, parser.parse_e) + string_and_int_expr.shouldBeValidPF { case e => + e.evaluateType(Map.empty) shouldBeValid WomPairType(WomArrayType(WomStringType), WomArrayType(WomIntegerType)) + } + + val int_and_int = """ unzip([(1,2),(3,4),(5,6)]) """ + val int_and_int_expr = fromString[ExpressionElement](int_and_int, parser.parse_e) + int_and_int_expr.shouldBeValidPF { case e => + e.evaluateType(Map.empty) shouldBeValid WomPairType(WomArrayType(WomIntegerType), WomArrayType(WomIntegerType)) + } + } } diff --git a/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluatorSpec.scala b/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluatorSpec.scala index 1487543a838..286f63e2881 100644 --- a/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluatorSpec.scala +++ b/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluatorSpec.scala @@ -11,7 +11,7 @@ import wdl.model.draft3.graph.expression.ValueEvaluator.ops._ import wdl.transforms.biscayne.Ast2WdlomSpec.{fromString, parser} import wdl.transforms.biscayne.ast2wdlom._ import wom.expression.NoIoFunctionSet -import wom.types.{WomIntegerType, WomMapType, WomOptionalType, WomStringType} +import wom.types.{WomArrayType, WomIntegerType, WomMapType, WomOptionalType, WomStringType} import wom.values.{WomArray, WomInteger, WomMap, WomOptionalValue, WomPair, WomString} class BiscayneValueEvaluatorSpec extends AnyFlatSpec with CromwellTimeoutSpec with Matchers { @@ -217,4 +217,30 @@ class BiscayneValueEvaluatorSpec extends AnyFlatSpec with CromwellTimeoutSpec wi e.evaluateValue(Map.empty, NoIoFunctionSet, None) shouldBeValid EvaluatedValue(expectedArray, Seq.empty) } } + + it should "evaluate an unzip expression correctly" in { + val str = """ unzip([("one", 1),("two", 2),("three", 3)]) """ + val expr = fromString[ExpressionElement](str, parser.parse_e) + + val left: WomArray = WomArray(WomArrayType(WomStringType), Seq(WomString("one"), WomString("two"), WomString("three"))) + val right: WomArray = WomArray(WomArrayType(WomIntegerType), Seq(WomInteger(1), WomInteger(2), WomInteger(3))) + val expectedPair: WomPair = WomPair(left, right) + + expr.shouldBeValidPF { case e => + e.evaluateValue(Map.empty, NoIoFunctionSet, None) shouldBeValid EvaluatedValue(expectedPair, Seq.empty) + } + } + + it should "evaluate an unzip on an empty collection correctly" in { + val str = """ unzip([])""" + val expr = fromString[ExpressionElement](str, parser.parse_e) + + val left: WomArray = WomArray(WomArrayType(WomStringType), Seq()) + val right: WomArray = WomArray(WomArrayType(WomIntegerType), Seq()) + val expectedPair: WomPair = WomPair(left, right) + + expr.shouldBeValidPF { case e => + e.evaluateValue(Map.empty, NoIoFunctionSet, None) shouldBeValid EvaluatedValue(expectedPair, Seq.empty) + } + } } From 67a2eea0215d173ba06fc4a1af4fac36575b447f Mon Sep 17 00:00:00 2001 From: Tom Wiseman Date: Wed, 21 Feb 2024 14:25:32 -0500 Subject: [PATCH 06/20] testing mostly complete --- .../expression/types/BiscayneTypeEvaluators.scala | 1 + .../expression/types/BiscayneTypeEvaluatorSpec.scala | 10 +++++++++- .../expression/values/BiscayneValueEvaluatorSpec.scala | 6 +++--- 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/types/BiscayneTypeEvaluators.scala b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/types/BiscayneTypeEvaluators.scala index d5ccaac30aa..3abb6c9856e 100644 --- a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/types/BiscayneTypeEvaluators.scala +++ b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/types/BiscayneTypeEvaluators.scala @@ -114,6 +114,7 @@ object BiscayneTypeEvaluators { implicit val unzipFunctionEvaluator: TypeEvaluator[Unzip] = new TypeEvaluator[Unzip]{ override def evaluateType(a: Unzip, linkedValues: Map[UnlinkedConsumedValueHook, GeneratedValueHandle])(implicit expressionTypeEvaluator: TypeEvaluator[ExpressionElement]) : ErrorOr[WomType] = validateParamType(a.param, linkedValues, WomArrayType(WomPairType(WomAnyType, WomAnyType))) flatMap { + case WomArrayType(WomNothingType) => WomPairType(WomArrayType(WomAnyType), WomArrayType(WomAnyType)).validNel case WomArrayType(WomPairType(x,y)) => WomPairType(WomArrayType(x), WomArrayType(y)).validNel case other => s"Cannot invoke 'unzip' on type '${other.stableName}'. Expected an array of pairs".invalidNel } diff --git a/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/types/BiscayneTypeEvaluatorSpec.scala b/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/types/BiscayneTypeEvaluatorSpec.scala index 43ee0931751..74020531c5f 100644 --- a/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/types/BiscayneTypeEvaluatorSpec.scala +++ b/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/types/BiscayneTypeEvaluatorSpec.scala @@ -65,7 +65,7 @@ class BiscayneTypeEvaluatorSpec extends AnyFlatSpec with CromwellTimeoutSpec wit } } - it should "evaluate the type of a unzip() function as Pair[Array[X], Array[Y]]" in { + it should "evaluate the type of an unzip() function as Pair[Array[X], Array[Y]]" in { val string_and_int = """ unzip([("one", 1),("two", 2),("three", 3)]) """ val string_and_int_expr = fromString[ExpressionElement](string_and_int, parser.parse_e) string_and_int_expr.shouldBeValidPF { case e => @@ -78,4 +78,12 @@ class BiscayneTypeEvaluatorSpec extends AnyFlatSpec with CromwellTimeoutSpec wit e.evaluateType(Map.empty) shouldBeValid WomPairType(WomArrayType(WomIntegerType), WomArrayType(WomIntegerType)) } } + + it should "evaluate the type of an unzip() function on an empty collection as Pair[Array[Any], Array[Any]]" in { + val empty = """ unzip([]) """ + val empty_unzip_expr = fromString[ExpressionElement](empty, parser.parse_e) + empty_unzip_expr.shouldBeValidPF { case e => + e.evaluateType(Map.empty) shouldBeValid WomPairType(WomArrayType(WomAnyType), WomArrayType(WomAnyType)) + } + } } diff --git a/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluatorSpec.scala b/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluatorSpec.scala index 286f63e2881..04e823c8fe5 100644 --- a/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluatorSpec.scala +++ b/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluatorSpec.scala @@ -11,7 +11,7 @@ import wdl.model.draft3.graph.expression.ValueEvaluator.ops._ import wdl.transforms.biscayne.Ast2WdlomSpec.{fromString, parser} import wdl.transforms.biscayne.ast2wdlom._ import wom.expression.NoIoFunctionSet -import wom.types.{WomArrayType, WomIntegerType, WomMapType, WomOptionalType, WomStringType} +import wom.types.{WomAnyType, WomArrayType, WomIntegerType, WomMapType, WomOptionalType, WomStringType} import wom.values.{WomArray, WomInteger, WomMap, WomOptionalValue, WomPair, WomString} class BiscayneValueEvaluatorSpec extends AnyFlatSpec with CromwellTimeoutSpec with Matchers { @@ -235,8 +235,8 @@ class BiscayneValueEvaluatorSpec extends AnyFlatSpec with CromwellTimeoutSpec wi val str = """ unzip([])""" val expr = fromString[ExpressionElement](str, parser.parse_e) - val left: WomArray = WomArray(WomArrayType(WomStringType), Seq()) - val right: WomArray = WomArray(WomArrayType(WomIntegerType), Seq()) + val left: WomArray = WomArray(WomArrayType(WomAnyType), Seq()) + val right: WomArray = WomArray(WomArrayType(WomAnyType), Seq()) val expectedPair: WomPair = WomPair(left, right) expr.shouldBeValidPF { case e => From ec039be652fddafcea38e8cd3d604ba57b52e2e6 Mon Sep 17 00:00:00 2001 From: Tom Wiseman Date: Wed, 21 Feb 2024 15:51:11 -0500 Subject: [PATCH 07/20] also do cascades --- .../values/BiscayneValueEvaluators.scala | 4 +-- .../AstToNewExpressionElements.scala | 3 +- .../CascadesExpressionValueConsumers.scala | 7 +++++ .../expression/consumed/consumed.scala | 1 + .../files/CascadesFileEvaluators.scala | 3 +- .../linking/expression/files/files.scala | 1 + .../types/CascadesTypeEvaluators.scala | 9 ++++++ .../linking/expression/types/types.scala | 1 + .../values/CascadesValueEvaluators.scala | 30 +++++++++++++++++++ .../linking/expression/values/values.scala | 2 ++ .../transforms/cascades/Ast2WdlomSpec.scala | 6 ++++ ...CascadesExpressionValueConsumersSpec.scala | 9 ++++++ .../files/CascadesFileEvaluatorSpec.scala | 11 +++++++ .../types/CascadesTypeEvaluatorSpec.scala | 8 +++++ .../values/CascadesValueEvaluatorSpec.scala | 27 ++++++++++++++++- .../transforms/ast2wdlom/Ast2WdlomSpec.scala | 6 ++++ 16 files changed, 123 insertions(+), 5 deletions(-) diff --git a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluators.scala b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluators.scala index d96f7481a8c..e288d1f8c07 100644 --- a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluators.scala +++ b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluators.scala @@ -236,9 +236,9 @@ object BiscayneValueEvaluators { } } - // Pair[Array[X], Array[Y]] unzip(Array[Pair[X, Y]]) - // Creates a Pair of Arrays, the first containing the elements from the left members of an Array of Pairs, and the second containing the right members. + // Creates a Pair of Arrays, the first containing the elements from the left members of an array of pairs, and the second containing the right members. // This is the inverse of the zip function. + // https://github.com/openwdl/wdl/blob/main/versions/1.1/SPEC.md#-pairarrayx-arrayy-unziparraypairx-y // @params : Array[Pair[X,Y]] // @returns : Pair[Array[X], Array[Y]] implicit val unzipFunctionEvaluator: ValueEvaluator[Unzip] = new ValueEvaluator[Unzip] { diff --git a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/ast2wdlom/AstToNewExpressionElements.scala b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/ast2wdlom/AstToNewExpressionElements.scala index 46cd8cb2e60..8e5443fa7da 100644 --- a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/ast2wdlom/AstToNewExpressionElements.scala +++ b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/ast2wdlom/AstToNewExpressionElements.scala @@ -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, Suffix} +import wdl.model.draft3.elements.ExpressionElement.{AsMap, AsPairs, CollectByKey, Keys, Max, Min, Sep, Suffix, Unzip} import wdl.transforms.base.ast2wdlom.AstNodeToExpressionElement object AstToNewExpressionElements { @@ -16,6 +16,7 @@ object AstToNewExpressionElements { "max" -> AstNodeToExpressionElement.validateTwoParamEngineFunction(Max, "max"), "sep" -> AstNodeToExpressionElement.validateTwoParamEngineFunction(Sep, "sep"), "suffix" -> AstNodeToExpressionElement.validateTwoParamEngineFunction(Suffix, "suffix"), + "unzip" -> AstNodeToExpressionElement.validateOneParamEngineFunction(Unzip, "unzip"), "read_object" -> (_ => "read_object is no longer available in this WDL version. Consider using read_json instead".invalidNel ), diff --git a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/consumed/CascadesExpressionValueConsumers.scala b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/consumed/CascadesExpressionValueConsumers.scala index ebc6d48fba1..32838814b3d 100644 --- a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/consumed/CascadesExpressionValueConsumers.scala +++ b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/consumed/CascadesExpressionValueConsumers.scala @@ -74,4 +74,11 @@ object cascadesExpressionValueConsumers { // None literals consume no values: Set.empty[UnlinkedConsumedValueHook] } + + implicit val unzipExpressionValueConsumer: ExpressionValueConsumer[Unzip] = new ExpressionValueConsumer[Unzip] { + override def expressionConsumedValueHooks(a: Unzip)(implicit + expressionValueConsumer: ExpressionValueConsumer[ExpressionElement] + ): Set[UnlinkedConsumedValueHook] = + expressionValueConsumer.expressionConsumedValueHooks(a.param)(expressionValueConsumer) + } } diff --git a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/consumed/consumed.scala b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/consumed/consumed.scala index 2026aaec010..72e8c8b4232 100644 --- a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/consumed/consumed.scala +++ b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/consumed/consumed.scala @@ -95,6 +95,7 @@ package object consumed { case a: Zip => a.expressionConsumedValueHooks(expressionValueConsumer) case a: Cross => a.expressionConsumedValueHooks(expressionValueConsumer) + case a: Unzip => a.expressionConsumedValueHooks(expressionValueConsumer) case a: Sub => a.expressionConsumedValueHooks(expressionValueConsumer) diff --git a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/files/CascadesFileEvaluators.scala b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/files/CascadesFileEvaluators.scala index 32ea447b584..12f4dcff101 100644 --- a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/files/CascadesFileEvaluators.scala +++ b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/files/CascadesFileEvaluators.scala @@ -1,6 +1,6 @@ package wdl.transforms.cascades.linking.expression.files -import wdl.model.draft3.elements.ExpressionElement.{AsMap, AsPairs, CollectByKey, Keys, Max, Min, Sep, Suffix} +import wdl.model.draft3.elements.ExpressionElement.{AsMap, AsPairs, CollectByKey, Keys, Max, Min, Sep, Suffix, Unzip} import wdl.model.draft3.graph.expression.FileEvaluator import wdl.transforms.base.linking.expression.files.EngineFunctionEvaluators import wdl.transforms.base.linking.expression.files.EngineFunctionEvaluators.twoParameterFunctionPassthroughFileEvaluator @@ -14,6 +14,7 @@ object cascadesFileEvaluators { EngineFunctionEvaluators.singleParameterPassthroughFileEvaluator implicit val collectByKeyFileEvaluator: FileEvaluator[CollectByKey] = EngineFunctionEvaluators.singleParameterPassthroughFileEvaluator + implicit val unzipFunctionEvaluator: FileEvaluator[Unzip] = EngineFunctionEvaluators.singleParameterPassthroughFileEvaluator implicit val sepFunctionEvaluator: FileEvaluator[Sep] = twoParameterFunctionPassthroughFileEvaluator[Sep] implicit val suffixFunctionEvaluator: FileEvaluator[Suffix] = twoParameterFunctionPassthroughFileEvaluator[Suffix] diff --git a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/files/files.scala b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/files/files.scala index a53c85b9b89..4fe05758d58 100644 --- a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/files/files.scala +++ b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/files/files.scala @@ -153,6 +153,7 @@ package object files { case a: Zip => a.predictFilesNeededToEvaluate(inputs, ioFunctionSet, coerceTo)(fileEvaluator, valueEvaluator) case a: Cross => a.predictFilesNeededToEvaluate(inputs, ioFunctionSet, coerceTo)(fileEvaluator, valueEvaluator) + case a: Unzip => a.predictFilesNeededToEvaluate(inputs, ioFunctionSet, coerceTo)(fileEvaluator, valueEvaluator) case a: Sub => a.predictFilesNeededToEvaluate(inputs, ioFunctionSet, coerceTo)(fileEvaluator, valueEvaluator) diff --git a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/types/CascadesTypeEvaluators.scala b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/types/CascadesTypeEvaluators.scala index 180af9eb0d8..4fba51a0f30 100644 --- a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/types/CascadesTypeEvaluators.scala +++ b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/types/CascadesTypeEvaluators.scala @@ -110,4 +110,13 @@ object cascadesTypeEvaluators { validateParamType(a.array, linkedValues, WomArrayType(WomStringType)) ) mapN { (_, _) => WomArrayType(WomStringType) } } + + implicit val unzipFunctionEvaluator: TypeEvaluator[Unzip] = new TypeEvaluator[Unzip]{ + override def evaluateType(a: Unzip, linkedValues: Map[UnlinkedConsumedValueHook, GeneratedValueHandle])(implicit expressionTypeEvaluator: TypeEvaluator[ExpressionElement]) : ErrorOr[WomType] = + validateParamType(a.param, linkedValues, WomArrayType(WomPairType(WomAnyType, WomAnyType))) flatMap { + case WomArrayType(WomNothingType) => WomPairType(WomArrayType(WomAnyType), WomArrayType(WomAnyType)).validNel + case WomArrayType(WomPairType(x,y)) => WomPairType(WomArrayType(x), WomArrayType(y)).validNel + case other => s"Cannot invoke 'unzip' on type '${other.stableName}'. Expected an array of pairs".invalidNel + } + } } diff --git a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/types/types.scala b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/types/types.scala index 4dd7c528e8b..d52c8e6d618 100644 --- a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/types/types.scala +++ b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/types/types.scala @@ -97,6 +97,7 @@ package object types { case a: Zip => a.evaluateType(linkedValues)(typeEvaluator) case a: Cross => a.evaluateType(linkedValues)(typeEvaluator) + case a: Unzip => a.evaluateType(linkedValues)(typeEvaluator) case a: Sub => a.evaluateType(linkedValues)(typeEvaluator) diff --git a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluators.scala b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluators.scala index db9df85bf06..190e5eff9d5 100644 --- a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluators.scala +++ b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluators.scala @@ -235,4 +235,34 @@ object cascadesValueEvaluators { EvaluatedValue(WomArray(arr.value.map(v => WomString(v.valueString + suffix.value))), Seq.empty).validNel } } + + // Creates a Pair of Arrays, the first containing the elements from the left members of an array of pairs, and the second containing the right members. + // This is the inverse of the zip function. + // https://github.com/openwdl/wdl/blob/main/versions/1.1/SPEC.md#-pairarrayx-arrayy-unziparraypairx-y + // @params : Array[Pair[X,Y]] + // @returns : Pair[Array[X], Array[Y]] + implicit val unzipFunctionEvaluator: ValueEvaluator[Unzip] = new ValueEvaluator[Unzip] { + override def evaluateValue(a: Unzip, + inputs: Map[String, WomValue], + ioFunctionSet: IoFunctionSet, + forCommandInstantiationOptions: Option[ForCommandInstantiationOptions] + )(implicit expressionValueEvaluator: ValueEvaluator[ExpressionElement]): ErrorOr[EvaluatedValue[WomPair]] = { + processValidatedSingleValue[WomArray, WomPair]( + expressionValueEvaluator.evaluateValue(a.param, inputs, ioFunctionSet, forCommandInstantiationOptions)( + expressionValueEvaluator + ) + ) { + case WomArray(WomArrayType(WomAnyType), Seq()) => EvaluatedValue(WomPair(WomArray(WomArrayType(WomAnyType), Seq.empty), WomArray(WomArrayType(WomAnyType), Seq.empty)),Seq.empty).validNel + case WomArray(WomArrayType(WomPairType(_,_)), values) => + val zippedPairs: Seq[(WomValue, WomValue)] = values map { case pair: WomPair => + Tuple2(pair.left, pair.right) + } + val (left, right) = zippedPairs.unzip + val unzippedPairs: WomPair = WomPair(WomArray(left), WomArray(right)) + EvaluatedValue(unzippedPairs, Seq.empty).validNel + case other => + s"Invalid call of 'unzip' on parameter of type '${other.womType.stableName}' (expected Array[Pair[X, Y]])".invalidNel + } + } + } } diff --git a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/values/values.scala b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/values/values.scala index 8850d4371df..cff3faab997 100644 --- a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/values/values.scala +++ b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/values/values.scala @@ -164,6 +164,8 @@ package object values { case a: Zip => a.evaluateValue(inputs, ioFunctionSet, forCommandInstantiationOptions)(expressionValueEvaluator) case a: Cross => a.evaluateValue(inputs, ioFunctionSet, forCommandInstantiationOptions)(expressionValueEvaluator) + case a: Unzip => + a.evaluateValue(inputs, ioFunctionSet, forCommandInstantiationOptions)(expressionValueEvaluator) case a: Sub => a.evaluateValue(inputs, ioFunctionSet, forCommandInstantiationOptions)(expressionValueEvaluator) diff --git a/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/Ast2WdlomSpec.scala b/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/Ast2WdlomSpec.scala index 88b7aff6578..196f7030087 100644 --- a/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/Ast2WdlomSpec.scala +++ b/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/Ast2WdlomSpec.scala @@ -103,4 +103,10 @@ class Ast2WdlomSpec extends AnyFlatSpec with CromwellTimeoutSpec with Matchers { val expr = fromString[ExpressionElement](str, parser.parse_e) expr shouldBeValid (Suffix(IdentifierLookup("some_str"), IdentifierLookup("some_arr"))) } + + it should "parse the new unzip function" in { + val str = "unzip(some_array_of_pairs)" + val expr = fromString[ExpressionElement](str, parser.parse_e) + expr shouldBeValid (Unzip(IdentifierLookup("some_array_of_pairs"))) + } } diff --git a/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/consumed/CascadesExpressionValueConsumersSpec.scala b/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/consumed/CascadesExpressionValueConsumersSpec.scala index 57beb7a12a1..13ecd0783c3 100644 --- a/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/consumed/CascadesExpressionValueConsumersSpec.scala +++ b/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/consumed/CascadesExpressionValueConsumersSpec.scala @@ -75,4 +75,13 @@ class CascadesExpressionValueConsumersSpec extends AnyFlatSpec with CromwellTime e.expressionConsumedValueHooks should be(Set(UnlinkedIdentifierHook("my_array"))) } } + + it should "discover an array variable lookup within a unzip() call" in { + val str = """ unzip(my_array_of_pairs) """ + val expr = fromString[ExpressionElement](str, parser.parse_e) + + expr.shouldBeValidPF { case e => + e.expressionConsumedValueHooks should be(Set(UnlinkedIdentifierHook("my_array_of_pairs"))) + } + } } diff --git a/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/files/CascadesFileEvaluatorSpec.scala b/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/files/CascadesFileEvaluatorSpec.scala index 5335f7d5f29..5546d2429fe 100644 --- a/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/files/CascadesFileEvaluatorSpec.scala +++ b/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/files/CascadesFileEvaluatorSpec.scala @@ -56,4 +56,15 @@ class CascadesFileEvaluatorSpec extends AnyFlatSpec with CromwellTimeoutSpec wit ) } } + + it should "discover the file which would be required to evaluate a unzip() function" in { + val str = """ unzip(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") + ) + } + } } diff --git a/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/types/CascadesTypeEvaluatorSpec.scala b/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/types/CascadesTypeEvaluatorSpec.scala index 954ef0f52f0..52b740fde5f 100644 --- a/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/types/CascadesTypeEvaluatorSpec.scala +++ b/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/types/CascadesTypeEvaluatorSpec.scala @@ -64,4 +64,12 @@ class CascadesTypeEvaluatorSpec extends AnyFlatSpec with CromwellTimeoutSpec wit e.evaluateType(Map.empty) shouldBeValid WomArrayType(WomStringType) } } + + it should "evaluate the type of an unzip() function on an empty collection as Pair[Array[Any], Array[Any]]" in { + val empty = """ unzip([]) """ + val empty_unzip_expr = fromString[ExpressionElement](empty, parser.parse_e) + empty_unzip_expr.shouldBeValidPF { case e => + e.evaluateType(Map.empty) shouldBeValid WomPairType(WomArrayType(WomAnyType), WomArrayType(WomAnyType)) + } + } } diff --git a/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluatorSpec.scala b/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluatorSpec.scala index bd47a20ca68..f4539df682e 100644 --- a/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluatorSpec.scala +++ b/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluatorSpec.scala @@ -11,7 +11,7 @@ import wdl.model.draft3.graph.expression.ValueEvaluator.ops._ import wdl.transforms.cascades.Ast2WdlomSpec.{fromString, parser} import wdl.transforms.cascades.ast2wdlom._ import wom.expression.NoIoFunctionSet -import wom.types.{WomIntegerType, WomMapType, WomOptionalType, WomStringType} +import wom.types.{WomAnyType, WomArrayType, WomIntegerType, WomMapType, WomOptionalType, WomStringType} import wom.values.{WomArray, WomInteger, WomMap, WomOptionalValue, WomPair, WomString} class CascadesValueEvaluatorSpec extends AnyFlatSpec with CromwellTimeoutSpec with Matchers { @@ -217,4 +217,29 @@ class CascadesValueEvaluatorSpec extends AnyFlatSpec with CromwellTimeoutSpec wi e.evaluateValue(Map.empty, NoIoFunctionSet, None) shouldBeValid EvaluatedValue(expectedArray, Seq.empty) } } + it should "evaluate an unzip expression correctly" in { + val str = """ unzip([("one", 1),("two", 2),("three", 3)]) """ + val expr = fromString[ExpressionElement](str, parser.parse_e) + + val left: WomArray = WomArray(WomArrayType(WomStringType), Seq(WomString("one"), WomString("two"), WomString("three"))) + val right: WomArray = WomArray(WomArrayType(WomIntegerType), Seq(WomInteger(1), WomInteger(2), WomInteger(3))) + val expectedPair: WomPair = WomPair(left, right) + + expr.shouldBeValidPF { case e => + e.evaluateValue(Map.empty, NoIoFunctionSet, None) shouldBeValid EvaluatedValue(expectedPair, Seq.empty) + } + } + + it should "evaluate an unzip on an empty collection correctly" in { + val str = """ unzip([])""" + val expr = fromString[ExpressionElement](str, parser.parse_e) + + val left: WomArray = WomArray(WomArrayType(WomAnyType), Seq()) + val right: WomArray = WomArray(WomArrayType(WomAnyType), Seq()) + val expectedPair: WomPair = WomPair(left, right) + + expr.shouldBeValidPF { case e => + e.evaluateValue(Map.empty, NoIoFunctionSet, None) shouldBeValid EvaluatedValue(expectedPair, Seq.empty) + } + } } diff --git a/wdl/transforms/draft3/src/test/scala/wdl/draft3/transforms/ast2wdlom/Ast2WdlomSpec.scala b/wdl/transforms/draft3/src/test/scala/wdl/draft3/transforms/ast2wdlom/Ast2WdlomSpec.scala index 664696ee86d..3c3ec3bb06d 100644 --- a/wdl/transforms/draft3/src/test/scala/wdl/draft3/transforms/ast2wdlom/Ast2WdlomSpec.scala +++ b/wdl/transforms/draft3/src/test/scala/wdl/draft3/transforms/ast2wdlom/Ast2WdlomSpec.scala @@ -63,4 +63,10 @@ class Ast2WdlomSpec extends AnyFlatSpec with CromwellTimeoutSpec with Matchers { val expr = fromString[ExpressionElement](str, parser.parse_e) expr shouldBeValid (IdentifierLookup("None")) } + + it should "not parse the new unzip function" in { + val str = "unzip(some_array_of_pairs)" + val expr = fromString[ExpressionElement](str, parser.parse_e) + expr shouldBeInvalid "Failed to parse expression (reason 1 of 1): Unknown engine function: 'unzip'" + } } From 696332180a576b3a9e3b8bdc96b464af5b3074ba Mon Sep 17 00:00:00 2001 From: Tom Wiseman Date: Wed, 21 Feb 2024 15:52:20 -0500 Subject: [PATCH 08/20] run formatter --- .../consumed/BiscayneExpressionValueConsumers.scala | 3 ++- .../expression/files/BiscayneFileEvaluators.scala | 3 ++- .../expression/types/BiscayneTypeEvaluators.scala | 8 +++++--- .../expression/values/BiscayneValueEvaluators.scala | 11 +++++++---- .../biscayne/linking/expression/values/values.scala | 3 ++- .../consumed/CascadesExpressionValueConsumers.scala | 2 +- .../expression/files/CascadesFileEvaluators.scala | 3 ++- .../expression/types/CascadesTypeEvaluators.scala | 8 +++++--- .../expression/values/CascadesValueEvaluators.scala | 11 +++++++---- 9 files changed, 33 insertions(+), 19 deletions(-) diff --git a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/consumed/BiscayneExpressionValueConsumers.scala b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/consumed/BiscayneExpressionValueConsumers.scala index 057af06a6b3..c60972c2c9e 100644 --- a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/consumed/BiscayneExpressionValueConsumers.scala +++ b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/consumed/BiscayneExpressionValueConsumers.scala @@ -68,7 +68,8 @@ object BiscayneExpressionValueConsumers { implicit val unzipExpressionValueConsumer: ExpressionValueConsumer[Unzip] = new ExpressionValueConsumer[Unzip] { override def expressionConsumedValueHooks(a: Unzip)(implicit - expressionValueConsumer: ExpressionValueConsumer[ExpressionElement]): Set[UnlinkedConsumedValueHook] = + expressionValueConsumer: ExpressionValueConsumer[ExpressionElement] + ): Set[UnlinkedConsumedValueHook] = expressionValueConsumer.expressionConsumedValueHooks(a.param)(expressionValueConsumer) } diff --git a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/files/BiscayneFileEvaluators.scala b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/files/BiscayneFileEvaluators.scala index accb2a9435b..ff04d32e72e 100644 --- a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/files/BiscayneFileEvaluators.scala +++ b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/files/BiscayneFileEvaluators.scala @@ -21,5 +21,6 @@ object BiscayneFileEvaluators { implicit val minFunctionEvaluator: FileEvaluator[Min] = twoParameterFunctionPassthroughFileEvaluator[Min] implicit val maxFunctionEvaluator: FileEvaluator[Max] = twoParameterFunctionPassthroughFileEvaluator[Max] - implicit val unzipFunctionEvaluator: FileEvaluator[Unzip] = EngineFunctionEvaluators.singleParameterPassthroughFileEvaluator + implicit val unzipFunctionEvaluator: FileEvaluator[Unzip] = + EngineFunctionEvaluators.singleParameterPassthroughFileEvaluator } diff --git a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/types/BiscayneTypeEvaluators.scala b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/types/BiscayneTypeEvaluators.scala index 3abb6c9856e..04302774f6f 100644 --- a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/types/BiscayneTypeEvaluators.scala +++ b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/types/BiscayneTypeEvaluators.scala @@ -111,11 +111,13 @@ object BiscayneTypeEvaluators { ) mapN { (_, _) => WomArrayType(WomStringType) } } - implicit val unzipFunctionEvaluator: TypeEvaluator[Unzip] = new TypeEvaluator[Unzip]{ - override def evaluateType(a: Unzip, linkedValues: Map[UnlinkedConsumedValueHook, GeneratedValueHandle])(implicit expressionTypeEvaluator: TypeEvaluator[ExpressionElement]) : ErrorOr[WomType] = + implicit val unzipFunctionEvaluator: TypeEvaluator[Unzip] = new TypeEvaluator[Unzip] { + override def evaluateType(a: Unzip, linkedValues: Map[UnlinkedConsumedValueHook, GeneratedValueHandle])(implicit + expressionTypeEvaluator: TypeEvaluator[ExpressionElement] + ): ErrorOr[WomType] = validateParamType(a.param, linkedValues, WomArrayType(WomPairType(WomAnyType, WomAnyType))) flatMap { case WomArrayType(WomNothingType) => WomPairType(WomArrayType(WomAnyType), WomArrayType(WomAnyType)).validNel - case WomArrayType(WomPairType(x,y)) => WomPairType(WomArrayType(x), WomArrayType(y)).validNel + case WomArrayType(WomPairType(x, y)) => WomPairType(WomArrayType(x), WomArrayType(y)).validNel case other => s"Cannot invoke 'unzip' on type '${other.stableName}'. Expected an array of pairs".invalidNel } } diff --git a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluators.scala b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluators.scala index e288d1f8c07..f703776c844 100644 --- a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluators.scala +++ b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluators.scala @@ -246,14 +246,18 @@ object BiscayneValueEvaluators { inputs: Map[String, WomValue], ioFunctionSet: IoFunctionSet, forCommandInstantiationOptions: Option[ForCommandInstantiationOptions] - )(implicit expressionValueEvaluator: ValueEvaluator[ExpressionElement]): ErrorOr[EvaluatedValue[WomPair]] = { + )(implicit expressionValueEvaluator: ValueEvaluator[ExpressionElement]): ErrorOr[EvaluatedValue[WomPair]] = processValidatedSingleValue[WomArray, WomPair]( expressionValueEvaluator.evaluateValue(a.param, inputs, ioFunctionSet, forCommandInstantiationOptions)( expressionValueEvaluator ) ) { - case WomArray(WomArrayType(WomAnyType), Seq()) => EvaluatedValue(WomPair(WomArray(WomArrayType(WomAnyType), Seq.empty), WomArray(WomArrayType(WomAnyType), Seq.empty)),Seq.empty).validNel - case WomArray(WomArrayType(WomPairType(_,_)), values) => + case WomArray(WomArrayType(WomAnyType), Seq()) => + EvaluatedValue( + WomPair(WomArray(WomArrayType(WomAnyType), Seq.empty), WomArray(WomArrayType(WomAnyType), Seq.empty)), + Seq.empty + ).validNel + case WomArray(WomArrayType(WomPairType(_, _)), values) => val zippedPairs: Seq[(WomValue, WomValue)] = values map { case pair: WomPair => Tuple2(pair.left, pair.right) } @@ -263,6 +267,5 @@ object BiscayneValueEvaluators { case other => s"Invalid call of 'unzip' on parameter of type '${other.womType.stableName}' (expected Array[Pair[X, Y]])".invalidNel } - } } } diff --git a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/values/values.scala b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/values/values.scala index 878456c5038..5f1c7ee749d 100644 --- a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/values/values.scala +++ b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/values/values.scala @@ -164,7 +164,8 @@ package object values { case a: Zip => a.evaluateValue(inputs, ioFunctionSet, forCommandInstantiationOptions)(expressionValueEvaluator) case a: Cross => a.evaluateValue(inputs, ioFunctionSet, forCommandInstantiationOptions)(expressionValueEvaluator) - case a: Unzip => a.evaluateValue(inputs, ioFunctionSet, forCommandInstantiationOptions)(expressionValueEvaluator) + case a: Unzip => + a.evaluateValue(inputs, ioFunctionSet, forCommandInstantiationOptions)(expressionValueEvaluator) case a: Sub => a.evaluateValue(inputs, ioFunctionSet, forCommandInstantiationOptions)(expressionValueEvaluator) diff --git a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/consumed/CascadesExpressionValueConsumers.scala b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/consumed/CascadesExpressionValueConsumers.scala index 32838814b3d..79297465415 100644 --- a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/consumed/CascadesExpressionValueConsumers.scala +++ b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/consumed/CascadesExpressionValueConsumers.scala @@ -77,7 +77,7 @@ object cascadesExpressionValueConsumers { implicit val unzipExpressionValueConsumer: ExpressionValueConsumer[Unzip] = new ExpressionValueConsumer[Unzip] { override def expressionConsumedValueHooks(a: Unzip)(implicit - expressionValueConsumer: ExpressionValueConsumer[ExpressionElement] + expressionValueConsumer: ExpressionValueConsumer[ExpressionElement] ): Set[UnlinkedConsumedValueHook] = expressionValueConsumer.expressionConsumedValueHooks(a.param)(expressionValueConsumer) } diff --git a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/files/CascadesFileEvaluators.scala b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/files/CascadesFileEvaluators.scala index 12f4dcff101..6f8af727e43 100644 --- a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/files/CascadesFileEvaluators.scala +++ b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/files/CascadesFileEvaluators.scala @@ -14,7 +14,8 @@ object cascadesFileEvaluators { EngineFunctionEvaluators.singleParameterPassthroughFileEvaluator implicit val collectByKeyFileEvaluator: FileEvaluator[CollectByKey] = EngineFunctionEvaluators.singleParameterPassthroughFileEvaluator - implicit val unzipFunctionEvaluator: FileEvaluator[Unzip] = EngineFunctionEvaluators.singleParameterPassthroughFileEvaluator + implicit val unzipFunctionEvaluator: FileEvaluator[Unzip] = + EngineFunctionEvaluators.singleParameterPassthroughFileEvaluator implicit val sepFunctionEvaluator: FileEvaluator[Sep] = twoParameterFunctionPassthroughFileEvaluator[Sep] implicit val suffixFunctionEvaluator: FileEvaluator[Suffix] = twoParameterFunctionPassthroughFileEvaluator[Suffix] diff --git a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/types/CascadesTypeEvaluators.scala b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/types/CascadesTypeEvaluators.scala index 4fba51a0f30..2a54bead566 100644 --- a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/types/CascadesTypeEvaluators.scala +++ b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/types/CascadesTypeEvaluators.scala @@ -111,11 +111,13 @@ object cascadesTypeEvaluators { ) mapN { (_, _) => WomArrayType(WomStringType) } } - implicit val unzipFunctionEvaluator: TypeEvaluator[Unzip] = new TypeEvaluator[Unzip]{ - override def evaluateType(a: Unzip, linkedValues: Map[UnlinkedConsumedValueHook, GeneratedValueHandle])(implicit expressionTypeEvaluator: TypeEvaluator[ExpressionElement]) : ErrorOr[WomType] = + implicit val unzipFunctionEvaluator: TypeEvaluator[Unzip] = new TypeEvaluator[Unzip] { + override def evaluateType(a: Unzip, linkedValues: Map[UnlinkedConsumedValueHook, GeneratedValueHandle])(implicit + expressionTypeEvaluator: TypeEvaluator[ExpressionElement] + ): ErrorOr[WomType] = validateParamType(a.param, linkedValues, WomArrayType(WomPairType(WomAnyType, WomAnyType))) flatMap { case WomArrayType(WomNothingType) => WomPairType(WomArrayType(WomAnyType), WomArrayType(WomAnyType)).validNel - case WomArrayType(WomPairType(x,y)) => WomPairType(WomArrayType(x), WomArrayType(y)).validNel + case WomArrayType(WomPairType(x, y)) => WomPairType(WomArrayType(x), WomArrayType(y)).validNel case other => s"Cannot invoke 'unzip' on type '${other.stableName}'. Expected an array of pairs".invalidNel } } diff --git a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluators.scala b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluators.scala index 190e5eff9d5..4c24cdce334 100644 --- a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluators.scala +++ b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluators.scala @@ -246,14 +246,18 @@ object cascadesValueEvaluators { inputs: Map[String, WomValue], ioFunctionSet: IoFunctionSet, forCommandInstantiationOptions: Option[ForCommandInstantiationOptions] - )(implicit expressionValueEvaluator: ValueEvaluator[ExpressionElement]): ErrorOr[EvaluatedValue[WomPair]] = { + )(implicit expressionValueEvaluator: ValueEvaluator[ExpressionElement]): ErrorOr[EvaluatedValue[WomPair]] = processValidatedSingleValue[WomArray, WomPair]( expressionValueEvaluator.evaluateValue(a.param, inputs, ioFunctionSet, forCommandInstantiationOptions)( expressionValueEvaluator ) ) { - case WomArray(WomArrayType(WomAnyType), Seq()) => EvaluatedValue(WomPair(WomArray(WomArrayType(WomAnyType), Seq.empty), WomArray(WomArrayType(WomAnyType), Seq.empty)),Seq.empty).validNel - case WomArray(WomArrayType(WomPairType(_,_)), values) => + case WomArray(WomArrayType(WomAnyType), Seq()) => + EvaluatedValue( + WomPair(WomArray(WomArrayType(WomAnyType), Seq.empty), WomArray(WomArrayType(WomAnyType), Seq.empty)), + Seq.empty + ).validNel + case WomArray(WomArrayType(WomPairType(_, _)), values) => val zippedPairs: Seq[(WomValue, WomValue)] = values map { case pair: WomPair => Tuple2(pair.left, pair.right) } @@ -263,6 +267,5 @@ object cascadesValueEvaluators { case other => s"Invalid call of 'unzip' on parameter of type '${other.womType.stableName}' (expected Array[Pair[X, Y]])".invalidNel } - } } } From c072ccc427370bc045a1fd48c5debfacf4a73ebb Mon Sep 17 00:00:00 2001 From: Tom Wiseman Date: Wed, 21 Feb 2024 16:08:36 -0500 Subject: [PATCH 09/20] () --- .../standardTestCases/biscayne_new_engine_functions.test | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/centaur/src/main/resources/standardTestCases/biscayne_new_engine_functions.test b/centaur/src/main/resources/standardTestCases/biscayne_new_engine_functions.test index 1d06c7a21b6..d150ac149f0 100644 --- a/centaur/src/main/resources/standardTestCases/biscayne_new_engine_functions.test +++ b/centaur/src/main/resources/standardTestCases/biscayne_new_engine_functions.test @@ -59,7 +59,7 @@ metadata { "outputs.biscayne_new_engine_functions.with_suffixes.1": "bbbS" "outputs.biscayne_new_engine_functions.with_suffixes.2": "cccS" - "outputs.biscayne_new_engine_functions.unzipped_a": ([],[]) + "outputs.biscayne_new_engine_functions.unzipped_a": () "outputs.biscayne_new_engine_functions.unzipped_b": (["A"],["a"]) "outputs.biscayne_new_engine_functions.unzipped_c": (["A","B"],["a","b"]) "outputs.biscayne_new_engine_functions.unzipped_d": (["A","B","C"],["a","b","c"]) From f04bc9edf951444432a80af7945cb4ab09ffb448 Mon Sep 17 00:00:00 2001 From: Tom Wiseman Date: Wed, 21 Feb 2024 16:36:16 -0500 Subject: [PATCH 10/20] centaur test formatting --- .../biscayne_new_engine_functions.test | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/centaur/src/main/resources/standardTestCases/biscayne_new_engine_functions.test b/centaur/src/main/resources/standardTestCases/biscayne_new_engine_functions.test index d150ac149f0..3cea97846ba 100644 --- a/centaur/src/main/resources/standardTestCases/biscayne_new_engine_functions.test +++ b/centaur/src/main/resources/standardTestCases/biscayne_new_engine_functions.test @@ -59,9 +59,17 @@ metadata { "outputs.biscayne_new_engine_functions.with_suffixes.1": "bbbS" "outputs.biscayne_new_engine_functions.with_suffixes.2": "cccS" - "outputs.biscayne_new_engine_functions.unzipped_a": () - "outputs.biscayne_new_engine_functions.unzipped_b": (["A"],["a"]) - "outputs.biscayne_new_engine_functions.unzipped_c": (["A","B"],["a","b"]) - "outputs.biscayne_new_engine_functions.unzipped_d": (["A","B","C"],["a","b","c"]) - "outputs.biscayne_new_engine_functions.unzipped_e": (["one","two","three"],[1.0,2.0,3.0]) + "outputs.biscayne_new_engine_functions.unzipped_a.left": [] + + "outputs.biscayne_new_engine_functions.unzipped_b.left.0": "A" + "outputs.biscayne_new_engine_functions.unzipped_b.right.0": "a" + + "outputs.biscayne_new_engine_functions.unzipped_c.left": ["A", "B"] + "outputs.biscayne_new_engine_functions.unzipped_c.right": ["a", "b"] + + "outputs.biscayne_new_engine_functions.unzipped_d.left": ["A", "B", "C"] + "outputs.biscayne_new_engine_functions.unzipped_d.right": ["a", "b", "c"] + + "outputs.biscayne_new_engine_functions.unzipped_e.left": ["one", "two", "three"] + "outputs.biscayne_new_engine_functions.unzipped_e.right": [1.0, 2.0, 3.0] } From 132d338a85c353df72d3730fe4b6b9ebf1388efe Mon Sep 17 00:00:00 2001 From: Tom Wiseman Date: Thu, 22 Feb 2024 10:01:04 -0500 Subject: [PATCH 11/20] scalafmt --- .../linking/expression/values/BiscayneValueEvaluatorSpec.scala | 3 ++- .../linking/expression/values/CascadesValueEvaluatorSpec.scala | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluatorSpec.scala b/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluatorSpec.scala index 04e823c8fe5..2fc6fc848bc 100644 --- a/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluatorSpec.scala +++ b/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluatorSpec.scala @@ -222,7 +222,8 @@ class BiscayneValueEvaluatorSpec extends AnyFlatSpec with CromwellTimeoutSpec wi val str = """ unzip([("one", 1),("two", 2),("three", 3)]) """ val expr = fromString[ExpressionElement](str, parser.parse_e) - val left: WomArray = WomArray(WomArrayType(WomStringType), Seq(WomString("one"), WomString("two"), WomString("three"))) + val left: WomArray = + WomArray(WomArrayType(WomStringType), Seq(WomString("one"), WomString("two"), WomString("three"))) val right: WomArray = WomArray(WomArrayType(WomIntegerType), Seq(WomInteger(1), WomInteger(2), WomInteger(3))) val expectedPair: WomPair = WomPair(left, right) diff --git a/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluatorSpec.scala b/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluatorSpec.scala index f4539df682e..d4d2fbb0555 100644 --- a/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluatorSpec.scala +++ b/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluatorSpec.scala @@ -221,7 +221,8 @@ class CascadesValueEvaluatorSpec extends AnyFlatSpec with CromwellTimeoutSpec wi val str = """ unzip([("one", 1),("two", 2),("three", 3)]) """ val expr = fromString[ExpressionElement](str, parser.parse_e) - val left: WomArray = WomArray(WomArrayType(WomStringType), Seq(WomString("one"), WomString("two"), WomString("three"))) + val left: WomArray = + WomArray(WomArrayType(WomStringType), Seq(WomString("one"), WomString("two"), WomString("three"))) val right: WomArray = WomArray(WomArrayType(WomIntegerType), Seq(WomInteger(1), WomInteger(2), WomInteger(3))) val expectedPair: WomPair = WomPair(left, right) From 6b93ef8c922bf614e2c921806a33da16860acb67 Mon Sep 17 00:00:00 2001 From: Tom Wiseman Date: Thu, 22 Feb 2024 10:22:01 -0500 Subject: [PATCH 12/20] tone down test --- .../biscayne_new_engine_functions.test | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/centaur/src/main/resources/standardTestCases/biscayne_new_engine_functions.test b/centaur/src/main/resources/standardTestCases/biscayne_new_engine_functions.test index 3cea97846ba..979d95b874e 100644 --- a/centaur/src/main/resources/standardTestCases/biscayne_new_engine_functions.test +++ b/centaur/src/main/resources/standardTestCases/biscayne_new_engine_functions.test @@ -59,17 +59,11 @@ metadata { "outputs.biscayne_new_engine_functions.with_suffixes.1": "bbbS" "outputs.biscayne_new_engine_functions.with_suffixes.2": "cccS" - "outputs.biscayne_new_engine_functions.unzipped_a.left": [] - "outputs.biscayne_new_engine_functions.unzipped_b.left.0": "A" "outputs.biscayne_new_engine_functions.unzipped_b.right.0": "a" - "outputs.biscayne_new_engine_functions.unzipped_c.left": ["A", "B"] - "outputs.biscayne_new_engine_functions.unzipped_c.right": ["a", "b"] - - "outputs.biscayne_new_engine_functions.unzipped_d.left": ["A", "B", "C"] - "outputs.biscayne_new_engine_functions.unzipped_d.right": ["a", "b", "c"] - - "outputs.biscayne_new_engine_functions.unzipped_e.left": ["one", "two", "three"] - "outputs.biscayne_new_engine_functions.unzipped_e.right": [1.0, 2.0, 3.0] + "outputs.biscayne_new_engine_functions.unzipped_c.left.0": "A" + "outputs.biscayne_new_engine_functions.unzipped_c.left.1": "B" + "outputs.biscayne_new_engine_functions.unzipped_c.right.0": "a" + "outputs.biscayne_new_engine_functions.unzipped_c.right.1": "b" } From 8377efdc1f01b1daca00967119ae2b942ed6521a Mon Sep 17 00:00:00 2001 From: Tom Wiseman Date: Thu, 22 Feb 2024 11:07:51 -0500 Subject: [PATCH 13/20] tweak centaur, wording tweaks --- .../biscayne_new_engine_functions.test | 15 +++++++++---- .../biscayne_new_engine_functions.wdl | 10 +++------ .../values/BiscayneValueEvaluators.scala | 20 +++++++++-------- .../values/CascadesValueEvaluators.scala | 22 ++++++++++--------- 4 files changed, 37 insertions(+), 30 deletions(-) diff --git a/centaur/src/main/resources/standardTestCases/biscayne_new_engine_functions.test b/centaur/src/main/resources/standardTestCases/biscayne_new_engine_functions.test index 979d95b874e..1f6e94cecaa 100644 --- a/centaur/src/main/resources/standardTestCases/biscayne_new_engine_functions.test +++ b/centaur/src/main/resources/standardTestCases/biscayne_new_engine_functions.test @@ -59,11 +59,18 @@ metadata { "outputs.biscayne_new_engine_functions.with_suffixes.1": "bbbS" "outputs.biscayne_new_engine_functions.with_suffixes.2": "cccS" + "outputs.biscayne_new_engine_functions.unzipped_a.left.0": "A" + "outputs.biscayne_new_engine_functions.unzipped_a.right.0": "a" + "outputs.biscayne_new_engine_functions.unzipped_b.left.0": "A" + "outputs.biscayne_new_engine_functions.unzipped_b.left.1": "B" "outputs.biscayne_new_engine_functions.unzipped_b.right.0": "a" + "outputs.biscayne_new_engine_functions.unzipped_b.right.1": "b" - "outputs.biscayne_new_engine_functions.unzipped_c.left.0": "A" - "outputs.biscayne_new_engine_functions.unzipped_c.left.1": "B" - "outputs.biscayne_new_engine_functions.unzipped_c.right.0": "a" - "outputs.biscayne_new_engine_functions.unzipped_c.right.1": "b" + "outputs.biscayne_new_engine_functions.unzipped_c.left.0": "one" + "outputs.biscayne_new_engine_functions.unzipped_c.left.1": "two" + "outputs.biscayne_new_engine_functions.unzipped_c.left.2": "three" + "outputs.biscayne_new_engine_functions.unzipped_c.right.0": 1.0 + "outputs.biscayne_new_engine_functions.unzipped_c.right.1": 2.0 + "outputs.biscayne_new_engine_functions.unzipped_c.right.2": 3.0 } diff --git a/centaur/src/main/resources/standardTestCases/wdl_biscayne/biscayne_new_engine_functions/biscayne_new_engine_functions.wdl b/centaur/src/main/resources/standardTestCases/wdl_biscayne/biscayne_new_engine_functions/biscayne_new_engine_functions.wdl index eb10edd6f83..1269f88e10f 100644 --- a/centaur/src/main/resources/standardTestCases/wdl_biscayne/biscayne_new_engine_functions/biscayne_new_engine_functions.wdl +++ b/centaur/src/main/resources/standardTestCases/wdl_biscayne/biscayne_new_engine_functions/biscayne_new_engine_functions.wdl @@ -26,11 +26,9 @@ workflow biscayne_new_engine_functions { # max float... near enough: Float maxFloat = 179769313000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000.0 - Array[Pair[String,String]] zipped_a = [] - Array[Pair[String,String]] zipped_b = [("A","a")] - Array[Pair[String,String]] zipped_c = [("A","a"),("B","b")] - Array[Pair[String,String]] zipped_d = [("A","a"),("B","b"),("C","c")] - Array[Pair[String,Float]] zipped_e = [("one",1.0),("two",2.0),("three",3.0)] + Array[Pair[String,String]] zipped_a = [("A", "a")] + Array[Pair[String,String]] zipped_b = [("A", "a"),("B", "b")] + Array[Pair[String,Float]] zipped_c = [("one", 1.0),("two", 2.0),("three", 3.0)] output { @@ -66,8 +64,6 @@ workflow biscayne_new_engine_functions { Pair[Array[String], Array[String]] unzipped_a = unzip(zipped_a) Pair[Array[String], Array[String]] unzipped_b = unzip(zipped_b) Pair[Array[String], Array[String]] unzipped_c = unzip(zipped_c) - Pair[Array[String], Array[String]] unzipped_d = unzip(zipped_d) - Pair[Array[String], Array[Float]] unzipped_e = unzip(zipped_e) } } diff --git a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluators.scala b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluators.scala index f703776c844..256b99a0d5d 100644 --- a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluators.scala +++ b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluators.scala @@ -236,11 +236,13 @@ object BiscayneValueEvaluators { } } - // Creates a Pair of Arrays, the first containing the elements from the left members of an array of pairs, and the second containing the right members. - // This is the inverse of the zip function. - // https://github.com/openwdl/wdl/blob/main/versions/1.1/SPEC.md#-pairarrayx-arrayy-unziparraypairx-y - // @params : Array[Pair[X,Y]] - // @returns : Pair[Array[X], Array[Y]] + /** + * Unzip: Creates a pair of arrays, the first containing the elements from the left members of an array of pairs, + * and the second containing the right members. This is the inverse of the zip function. + * https://github.com/openwdl/wdl/blob/main/versions/1.1/SPEC.md#-pairarrayx-arrayy-unziparraypairx-y + * input: Array[Pair[X,Y]] + * output: Pair[Array[X], Array[Y]] + */ implicit val unzipFunctionEvaluator: ValueEvaluator[Unzip] = new ValueEvaluator[Unzip] { override def evaluateValue(a: Unzip, inputs: Map[String, WomValue], @@ -258,12 +260,12 @@ object BiscayneValueEvaluators { Seq.empty ).validNel case WomArray(WomArrayType(WomPairType(_, _)), values) => - val zippedPairs: Seq[(WomValue, WomValue)] = values map { case pair: WomPair => + val zippedArrayOfPairs: Seq[(WomValue, WomValue)] = values map { case pair: WomPair => Tuple2(pair.left, pair.right) } - val (left, right) = zippedPairs.unzip - val unzippedPairs: WomPair = WomPair(WomArray(left), WomArray(right)) - EvaluatedValue(unzippedPairs, Seq.empty).validNel + val (left, right) = zippedArrayOfPairs.unzip + val unzippedPairOfArrays: WomPair = WomPair(WomArray(left), WomArray(right)) + EvaluatedValue(unzippedPairOfArrays, Seq.empty).validNel case other => s"Invalid call of 'unzip' on parameter of type '${other.womType.stableName}' (expected Array[Pair[X, Y]])".invalidNel } diff --git a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluators.scala b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluators.scala index 4c24cdce334..78294d28222 100644 --- a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluators.scala +++ b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluators.scala @@ -236,17 +236,19 @@ object cascadesValueEvaluators { } } - // Creates a Pair of Arrays, the first containing the elements from the left members of an array of pairs, and the second containing the right members. - // This is the inverse of the zip function. - // https://github.com/openwdl/wdl/blob/main/versions/1.1/SPEC.md#-pairarrayx-arrayy-unziparraypairx-y - // @params : Array[Pair[X,Y]] - // @returns : Pair[Array[X], Array[Y]] + /** + * Unzip: Creates a pair of arrays, the first containing the elements from the left members of an array of pairs, + * and the second containing the right members. This is the inverse of the zip function. + * https://github.com/openwdl/wdl/blob/main/versions/1.1/SPEC.md#-pairarrayx-arrayy-unziparraypairx-y + * input: Array[Pair[X,Y]] + * output: Pair[Array[X], Array[Y]] + */ implicit val unzipFunctionEvaluator: ValueEvaluator[Unzip] = new ValueEvaluator[Unzip] { override def evaluateValue(a: Unzip, inputs: Map[String, WomValue], ioFunctionSet: IoFunctionSet, forCommandInstantiationOptions: Option[ForCommandInstantiationOptions] - )(implicit expressionValueEvaluator: ValueEvaluator[ExpressionElement]): ErrorOr[EvaluatedValue[WomPair]] = + )(implicit expressionValueEvaluator: ValueEvaluator[ExpressionElement]): ErrorOr[EvaluatedValue[WomPair]] = processValidatedSingleValue[WomArray, WomPair]( expressionValueEvaluator.evaluateValue(a.param, inputs, ioFunctionSet, forCommandInstantiationOptions)( expressionValueEvaluator @@ -258,12 +260,12 @@ object cascadesValueEvaluators { Seq.empty ).validNel case WomArray(WomArrayType(WomPairType(_, _)), values) => - val zippedPairs: Seq[(WomValue, WomValue)] = values map { case pair: WomPair => + val zippedArrayOfPairs: Seq[(WomValue, WomValue)] = values map { case pair: WomPair => Tuple2(pair.left, pair.right) } - val (left, right) = zippedPairs.unzip - val unzippedPairs: WomPair = WomPair(WomArray(left), WomArray(right)) - EvaluatedValue(unzippedPairs, Seq.empty).validNel + val (left, right) = zippedArrayOfPairs.unzip + val unzippedPairOfArrays: WomPair = WomPair(WomArray(left), WomArray(right)) + EvaluatedValue(unzippedPairOfArrays, Seq.empty).validNel case other => s"Invalid call of 'unzip' on parameter of type '${other.womType.stableName}' (expected Array[Pair[X, Y]])".invalidNel } From 9a4aa153a2aa0c7a25d94cf1d99798a99a52ab50 Mon Sep 17 00:00:00 2001 From: Tom Wiseman Date: Thu, 22 Feb 2024 14:20:14 -0500 Subject: [PATCH 14/20] scalafmt --- .../linking/expression/values/CascadesValueEvaluators.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluators.scala b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluators.scala index 78294d28222..4396bce01a2 100644 --- a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluators.scala +++ b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluators.scala @@ -248,7 +248,7 @@ object cascadesValueEvaluators { inputs: Map[String, WomValue], ioFunctionSet: IoFunctionSet, forCommandInstantiationOptions: Option[ForCommandInstantiationOptions] - )(implicit expressionValueEvaluator: ValueEvaluator[ExpressionElement]): ErrorOr[EvaluatedValue[WomPair]] = + )(implicit expressionValueEvaluator: ValueEvaluator[ExpressionElement]): ErrorOr[EvaluatedValue[WomPair]] = processValidatedSingleValue[WomArray, WomPair]( expressionValueEvaluator.evaluateValue(a.param, inputs, ioFunctionSet, forCommandInstantiationOptions)( expressionValueEvaluator From d0a5edcca02dc3424a53ef4cfe5a3804bf2473da Mon Sep 17 00:00:00 2001 From: Tom Wiseman Date: Fri, 23 Feb 2024 13:23:50 -0500 Subject: [PATCH 15/20] pr feedback --- .../values/BiscayneValueEvaluatorSpec.scala | 12 ++++++++++++ .../values/CascadesValueEvaluatorSpec.scala | 13 +++++++++++++ .../base/ast2wdlom/AstNodeToExpressionElement.scala | 2 +- 3 files changed, 26 insertions(+), 1 deletion(-) diff --git a/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluatorSpec.scala b/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluatorSpec.scala index 2fc6fc848bc..c5d8335170d 100644 --- a/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluatorSpec.scala +++ b/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluatorSpec.scala @@ -244,4 +244,16 @@ class BiscayneValueEvaluatorSpec extends AnyFlatSpec with CromwellTimeoutSpec wi e.evaluateValue(Map.empty, NoIoFunctionSet, None) shouldBeValid EvaluatedValue(expectedPair, Seq.empty) } } + + it should "fail to evaluate unzip on invalid pair" in { + val invalidPair = """ unzip([()])""" + val invalidPairExpr = fromString[ExpressionElement](invalidPair, parser.parse_e) + invalidPairExpr.shouldBeInvalid("Failed to parse expression (reason 1 of 1): No WDL support for 0-tuples") + } + + it should "fail to evaluate unzip on heterogeneous pairs" in { + val invalidPair = """ unzip([ (1, 11.0), ([1,2,3], 2.0) ])""" + val invalidPairExpr = fromString[ExpressionElement](invalidPair, parser.parse_e) + invalidPairExpr.map(e => e.evaluateValue(Map.empty, NoIoFunctionSet, None).shouldBeInvalid("Could not construct array of type WomMaybeEmptyArrayType(WomPairType(WomIntegerType,WomFloatType)) with this value: List(WomPair(WomInteger(1),WomFloat(11.0)), WomPair([1, 2, 3],WomFloat(2.0)))")) + } } diff --git a/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluatorSpec.scala b/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluatorSpec.scala index d4d2fbb0555..4f6f09a5bf5 100644 --- a/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluatorSpec.scala +++ b/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluatorSpec.scala @@ -217,6 +217,7 @@ class CascadesValueEvaluatorSpec extends AnyFlatSpec with CromwellTimeoutSpec wi e.evaluateValue(Map.empty, NoIoFunctionSet, None) shouldBeValid EvaluatedValue(expectedArray, Seq.empty) } } + it should "evaluate an unzip expression correctly" in { val str = """ unzip([("one", 1),("two", 2),("three", 3)]) """ val expr = fromString[ExpressionElement](str, parser.parse_e) @@ -243,4 +244,16 @@ class CascadesValueEvaluatorSpec extends AnyFlatSpec with CromwellTimeoutSpec wi e.evaluateValue(Map.empty, NoIoFunctionSet, None) shouldBeValid EvaluatedValue(expectedPair, Seq.empty) } } + + it should "fail to evaluate unzip on invalid pair" in { + val invalidPair = """ unzip([()])""" + val invalidPairExpr = fromString[ExpressionElement](invalidPair, parser.parse_e) + invalidPairExpr.shouldBeInvalid("Failed to parse expression (reason 1 of 1): No WDL support for 0-tuples") + } + + it should "fail to evaluate unzip on heterogeneous pairs" in { + val invalidPair = """ unzip([ (1, 11.0), ([1,2,3], 2.0) ])""" + val invalidPairExpr = fromString[ExpressionElement](invalidPair, parser.parse_e) + invalidPairExpr.map(e => e.evaluateValue(Map.empty, NoIoFunctionSet, None).shouldBeInvalid("Could not construct array of type WomMaybeEmptyArrayType(WomPairType(WomIntegerType,WomFloatType)) with this value: List(WomPair(WomInteger(1),WomFloat(11.0)), WomPair([1, 2, 3],WomFloat(2.0)))")) + } } diff --git a/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/ast2wdlom/AstNodeToExpressionElement.scala b/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/ast2wdlom/AstNodeToExpressionElement.scala index 2a773fd4e23..ec01bdddd74 100644 --- a/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/ast2wdlom/AstNodeToExpressionElement.scala +++ b/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/ast2wdlom/AstNodeToExpressionElement.scala @@ -43,7 +43,7 @@ object AstNodeToExpressionElement { (a.getAttributeAsVector[ExpressionElement]("values") flatMap { case pair if pair.length == 2 => PairLiteral(pair.head, pair(1)).validNelCheck case singleton if singleton.length == 1 => singleton.head.validNelCheck - case more => s"No WDL support for ${more.size}-tuples in draft 3".invalidNelCheck + case more => s"No WDL support for ${more.size}-tuples".invalidNelCheck }).toValidated case a: GenericAst if a.getName == "ArrayLiteral" => a.getAttributeAsVector[ExpressionElement]("values").toValidated.map(ArrayLiteral) From 4a59ec99868a49e062d408204b5139027439675b Mon Sep 17 00:00:00 2001 From: Tom Wiseman Date: Fri, 23 Feb 2024 14:05:30 -0500 Subject: [PATCH 16/20] scalafmt --- .../expression/values/BiscayneValueEvaluatorSpec.scala | 7 ++++++- .../expression/values/CascadesValueEvaluatorSpec.scala | 7 ++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluatorSpec.scala b/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluatorSpec.scala index c5d8335170d..69c42d925b9 100644 --- a/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluatorSpec.scala +++ b/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluatorSpec.scala @@ -254,6 +254,11 @@ class BiscayneValueEvaluatorSpec extends AnyFlatSpec with CromwellTimeoutSpec wi it should "fail to evaluate unzip on heterogeneous pairs" in { val invalidPair = """ unzip([ (1, 11.0), ([1,2,3], 2.0) ])""" val invalidPairExpr = fromString[ExpressionElement](invalidPair, parser.parse_e) - invalidPairExpr.map(e => e.evaluateValue(Map.empty, NoIoFunctionSet, None).shouldBeInvalid("Could not construct array of type WomMaybeEmptyArrayType(WomPairType(WomIntegerType,WomFloatType)) with this value: List(WomPair(WomInteger(1),WomFloat(11.0)), WomPair([1, 2, 3],WomFloat(2.0)))")) + invalidPairExpr.map(e => + e.evaluateValue(Map.empty, NoIoFunctionSet, None) + .shouldBeInvalid( + "Could not construct array of type WomMaybeEmptyArrayType(WomPairType(WomIntegerType,WomFloatType)) with this value: List(WomPair(WomInteger(1),WomFloat(11.0)), WomPair([1, 2, 3],WomFloat(2.0)))" + ) + ) } } diff --git a/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluatorSpec.scala b/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluatorSpec.scala index 4f6f09a5bf5..add40615793 100644 --- a/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluatorSpec.scala +++ b/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluatorSpec.scala @@ -254,6 +254,11 @@ class CascadesValueEvaluatorSpec extends AnyFlatSpec with CromwellTimeoutSpec wi it should "fail to evaluate unzip on heterogeneous pairs" in { val invalidPair = """ unzip([ (1, 11.0), ([1,2,3], 2.0) ])""" val invalidPairExpr = fromString[ExpressionElement](invalidPair, parser.parse_e) - invalidPairExpr.map(e => e.evaluateValue(Map.empty, NoIoFunctionSet, None).shouldBeInvalid("Could not construct array of type WomMaybeEmptyArrayType(WomPairType(WomIntegerType,WomFloatType)) with this value: List(WomPair(WomInteger(1),WomFloat(11.0)), WomPair([1, 2, 3],WomFloat(2.0)))")) + invalidPairExpr.map(e => + e.evaluateValue(Map.empty, NoIoFunctionSet, None) + .shouldBeInvalid( + "Could not construct array of type WomMaybeEmptyArrayType(WomPairType(WomIntegerType,WomFloatType)) with this value: List(WomPair(WomInteger(1),WomFloat(11.0)), WomPair([1, 2, 3],WomFloat(2.0)))" + ) + ) } } From 7a6e979f10cd5d9cc5b7542d6ab291bc55c98cfb Mon Sep 17 00:00:00 2001 From: Tom Wiseman Date: Tue, 27 Feb 2024 09:43:18 -0500 Subject: [PATCH 17/20] add missing test --- .../values/CascadesValueEvaluatorSpec.scala | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluatorSpec.scala b/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluatorSpec.scala index add40615793..a459bae95e2 100644 --- a/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluatorSpec.scala +++ b/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluatorSpec.scala @@ -232,6 +232,20 @@ class CascadesValueEvaluatorSpec extends AnyFlatSpec with CromwellTimeoutSpec wi } } + it should "evaluate an unzip expression correctly" in { + val str = """ unzip([("one", 1),("two", 2),("three", 3)]) """ + val expr = fromString[ExpressionElement](str, parser.parse_e) + + val left: WomArray = + WomArray(WomArrayType(WomStringType), Seq(WomString("one"), WomString("two"), WomString("three"))) + val right: WomArray = WomArray(WomArrayType(WomIntegerType), Seq(WomInteger(1), WomInteger(2), WomInteger(3))) + val expectedPair: WomPair = WomPair(left, right) + + expr.shouldBeValidPF { case e => + e.evaluateValue(Map.empty, NoIoFunctionSet, None) shouldBeValid EvaluatedValue(expectedPair, Seq.empty) + } + } + it should "evaluate an unzip on an empty collection correctly" in { val str = """ unzip([])""" val expr = fromString[ExpressionElement](str, parser.parse_e) From 08378af4bf2244b5fab47d8575a0862701bc6da1 Mon Sep 17 00:00:00 2001 From: Tom Wiseman Date: Tue, 27 Feb 2024 09:54:55 -0500 Subject: [PATCH 18/20] add missing test --- .../types/CascadesTypeEvaluatorSpec.scala | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/types/CascadesTypeEvaluatorSpec.scala b/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/types/CascadesTypeEvaluatorSpec.scala index 52b740fde5f..ff2582b377d 100644 --- a/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/types/CascadesTypeEvaluatorSpec.scala +++ b/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/types/CascadesTypeEvaluatorSpec.scala @@ -65,6 +65,20 @@ class CascadesTypeEvaluatorSpec extends AnyFlatSpec with CromwellTimeoutSpec wit } } + it should "evaluate the type of an unzip() function as Pair[Array[X], Array[Y]]" in { + val string_and_int = """ unzip([("one", 1),("two", 2),("three", 3)]) """ + val string_and_int_expr = fromString[ExpressionElement](string_and_int, parser.parse_e) + string_and_int_expr.shouldBeValidPF { case e => + e.evaluateType(Map.empty) shouldBeValid WomPairType(WomArrayType(WomStringType), WomArrayType(WomIntegerType)) + } + + val int_and_int = """ unzip([(1,2),(3,4),(5,6)]) """ + val int_and_int_expr = fromString[ExpressionElement](int_and_int, parser.parse_e) + int_and_int_expr.shouldBeValidPF { case e => + e.evaluateType(Map.empty) shouldBeValid WomPairType(WomArrayType(WomIntegerType), WomArrayType(WomIntegerType)) + } + } + it should "evaluate the type of an unzip() function on an empty collection as Pair[Array[Any], Array[Any]]" in { val empty = """ unzip([]) """ val empty_unzip_expr = fromString[ExpressionElement](empty, parser.parse_e) From a8e2a4ccfabc366067238012c4a477ca0136915d Mon Sep 17 00:00:00 2001 From: Tom Wiseman Date: Tue, 27 Feb 2024 10:37:32 -0500 Subject: [PATCH 19/20] duplicate --- .../values/CascadesValueEvaluatorSpec.scala | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluatorSpec.scala b/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluatorSpec.scala index a459bae95e2..add40615793 100644 --- a/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluatorSpec.scala +++ b/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluatorSpec.scala @@ -232,20 +232,6 @@ class CascadesValueEvaluatorSpec extends AnyFlatSpec with CromwellTimeoutSpec wi } } - it should "evaluate an unzip expression correctly" in { - val str = """ unzip([("one", 1),("two", 2),("three", 3)]) """ - val expr = fromString[ExpressionElement](str, parser.parse_e) - - val left: WomArray = - WomArray(WomArrayType(WomStringType), Seq(WomString("one"), WomString("two"), WomString("three"))) - val right: WomArray = WomArray(WomArrayType(WomIntegerType), Seq(WomInteger(1), WomInteger(2), WomInteger(3))) - val expectedPair: WomPair = WomPair(left, right) - - expr.shouldBeValidPF { case e => - e.evaluateValue(Map.empty, NoIoFunctionSet, None) shouldBeValid EvaluatedValue(expectedPair, Seq.empty) - } - } - it should "evaluate an unzip on an empty collection correctly" in { val str = """ unzip([])""" val expr = fromString[ExpressionElement](str, parser.parse_e) From febc8cf5d76315191f524b26f6e8fc5bdb344770 Mon Sep 17 00:00:00 2001 From: Tom Wiseman Date: Tue, 27 Feb 2024 14:25:25 -0500 Subject: [PATCH 20/20] merge broke scalafmt --- .../ast2wdlom/AstToNewExpressionElements.scala | 13 ++++++++++++- .../expression/files/BiscayneFileEvaluators.scala | 13 ++++++++++++- .../ast2wdlom/AstToNewExpressionElements.scala | 13 ++++++++++++- .../expression/files/CascadesFileEvaluators.scala | 13 ++++++++++++- 4 files changed, 48 insertions(+), 4 deletions(-) diff --git a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/ast2wdlom/AstToNewExpressionElements.scala b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/ast2wdlom/AstToNewExpressionElements.scala index 87d73fc1e4c..f814044d1d0 100644 --- a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/ast2wdlom/AstToNewExpressionElements.scala +++ b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/ast2wdlom/AstToNewExpressionElements.scala @@ -3,7 +3,18 @@ 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, SubPosix, Suffix, Unzip} +import wdl.model.draft3.elements.ExpressionElement.{ + AsMap, + AsPairs, + CollectByKey, + Keys, + Max, + Min, + Sep, + SubPosix, + Suffix, + Unzip +} import wdl.transforms.base.ast2wdlom.AstNodeToExpressionElement object AstToNewExpressionElements { diff --git a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/files/BiscayneFileEvaluators.scala b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/files/BiscayneFileEvaluators.scala index 71ada3c80c9..89f830f81ab 100644 --- a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/files/BiscayneFileEvaluators.scala +++ b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/files/BiscayneFileEvaluators.scala @@ -1,6 +1,17 @@ package wdl.transforms.biscayne.linking.expression.files -import wdl.model.draft3.elements.ExpressionElement.{AsMap, AsPairs, CollectByKey, Keys, Max, Min, Sep, SubPosix, Suffix, Unzip} +import wdl.model.draft3.elements.ExpressionElement.{ + AsMap, + AsPairs, + CollectByKey, + Keys, + Max, + Min, + Sep, + SubPosix, + Suffix, + Unzip +} import wdl.model.draft3.graph.expression.FileEvaluator import wdl.transforms.base.linking.expression.files.EngineFunctionEvaluators import wdl.transforms.base.linking.expression.files.EngineFunctionEvaluators.{ diff --git a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/ast2wdlom/AstToNewExpressionElements.scala b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/ast2wdlom/AstToNewExpressionElements.scala index 6835d701ec8..1a2ce0d5735 100644 --- a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/ast2wdlom/AstToNewExpressionElements.scala +++ b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/ast2wdlom/AstToNewExpressionElements.scala @@ -3,7 +3,18 @@ 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, SubPosix, Suffix, Unzip} +import wdl.model.draft3.elements.ExpressionElement.{ + AsMap, + AsPairs, + CollectByKey, + Keys, + Max, + Min, + Sep, + SubPosix, + Suffix, + Unzip +} import wdl.transforms.base.ast2wdlom.AstNodeToExpressionElement object AstToNewExpressionElements { diff --git a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/files/CascadesFileEvaluators.scala b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/files/CascadesFileEvaluators.scala index 52e30325d51..7a1114f61b8 100644 --- a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/files/CascadesFileEvaluators.scala +++ b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/files/CascadesFileEvaluators.scala @@ -1,6 +1,17 @@ package wdl.transforms.cascades.linking.expression.files -import wdl.model.draft3.elements.ExpressionElement.{AsMap, AsPairs, CollectByKey, Keys, Max, Min, Sep, SubPosix, Suffix, Unzip} +import wdl.model.draft3.elements.ExpressionElement.{ + AsMap, + AsPairs, + CollectByKey, + Keys, + Max, + Min, + Sep, + SubPosix, + Suffix, + Unzip +} import wdl.model.draft3.graph.expression.FileEvaluator import wdl.transforms.base.linking.expression.files.EngineFunctionEvaluators import wdl.transforms.base.linking.expression.files.EngineFunctionEvaluators.{