From f05cac2214a1c64ed5a1dbf592028c722c5ec5b2 Mon Sep 17 00:00:00 2001 From: Miguel Covarrubias Date: Tue, 28 Feb 2023 08:08:17 -0500 Subject: [PATCH 1/3] wip --- .../read_write_map/read_write_map.wdl | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/centaur/src/main/resources/standardTestCases/read_write_map/read_write_map.wdl b/centaur/src/main/resources/standardTestCases/read_write_map/read_write_map.wdl index db04711e6c9..ef93957d9bd 100644 --- a/centaur/src/main/resources/standardTestCases/read_write_map/read_write_map.wdl +++ b/centaur/src/main/resources/standardTestCases/read_write_map/read_write_map.wdl @@ -26,10 +26,29 @@ task read_map { } } +task assert_write_map_terminal_newline { + Map[String, String] file_to_name + command <<< + # https://stackoverflow.com/a/25749716/21269164 + file_ends_with_newline() { + [[ $(tail -c1 "$1" | wc -l) -gt 0 ]] + } + + if ! file_ends_with_newline ${write_map(file_to_name)} + echo >&2 "write_map() should write a file whose last character is a newline" + exit 1 + fi + >>> + runtime { + docker: "python:3.5.0" + } +} + workflow wf { Map[String, String] map = {"f1": "alice", "f2": "bob", "f3": "chuck"} call write_map {input: file_to_name = map} call read_map + call assert_write_map_terminal_newline {input: file_name_to_map = map} output { Map[String, Int] out = read_map.out_map String contents = write_map.contents From cdf906b3536db018e50f51ba7d8f88bc807b4dd6 Mon Sep 17 00:00:00 2001 From: Miguel Covarrubias Date: Tue, 28 Feb 2023 08:51:48 -0500 Subject: [PATCH 2/3] write_map() should write the last entry with a newline --- .../standardTestCases/read_write_map/read_write_map.wdl | 7 ++++--- wom/src/main/scala/wom/values/WomMap.scala | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/centaur/src/main/resources/standardTestCases/read_write_map/read_write_map.wdl b/centaur/src/main/resources/standardTestCases/read_write_map/read_write_map.wdl index ef93957d9bd..151637332be 100644 --- a/centaur/src/main/resources/standardTestCases/read_write_map/read_write_map.wdl +++ b/centaur/src/main/resources/standardTestCases/read_write_map/read_write_map.wdl @@ -35,8 +35,9 @@ task assert_write_map_terminal_newline { } if ! file_ends_with_newline ${write_map(file_to_name)} - echo >&2 "write_map() should write a file whose last character is a newline" - exit 1 + then + echo >&2 "Error: write_map() should write a file whose last character is a newline" + exit 1 fi >>> runtime { @@ -48,7 +49,7 @@ workflow wf { Map[String, String] map = {"f1": "alice", "f2": "bob", "f3": "chuck"} call write_map {input: file_to_name = map} call read_map - call assert_write_map_terminal_newline {input: file_name_to_map = map} + call assert_write_map_terminal_newline {input: file_to_name = map} output { Map[String, Int] out = read_map.out_map String contents = write_map.contents diff --git a/wom/src/main/scala/wom/values/WomMap.scala b/wom/src/main/scala/wom/values/WomMap.scala index 6c0ada18849..bffd26d9e49 100644 --- a/wom/src/main/scala/wom/values/WomMap.scala +++ b/wom/src/main/scala/wom/values/WomMap.scala @@ -70,7 +70,7 @@ final case class WomMap private(womType: WomMapType, value: Map[WomValue, WomVal def tsvSerialize: Try[String] = { (womType.keyType, womType.valueType) match { case (_: WomPrimitiveType, _: WomPrimitiveType) => - Success(value.map({case (k, v) => s"${k.valueString}\t${v.valueString}"}).mkString("\n")) + Success(value.map({case (k, v) => s"${k.valueString}\t${v.valueString}"}).mkString(start="", sep="\n", end="\n")) case _ => Failure(new UnsupportedOperationException("Can only TSV serialize a Map[Primitive, Primitive]")) } From 8977a2deea7b1716907bb4b23675e48d2f722688 Mon Sep 17 00:00:00 2001 From: Miguel Covarrubias Date: Wed, 1 Mar 2023 08:11:53 -0500 Subject: [PATCH 3/3] PR feedback --- .../read_write_map/read_write_map.wdl | 20 ------------- wom/src/main/scala/wom/values/WomMap.scala | 6 ++++ .../test/scala/wom/types/WomMapTypeSpec.scala | 29 +++++++++++++++++++ 3 files changed, 35 insertions(+), 20 deletions(-) diff --git a/centaur/src/main/resources/standardTestCases/read_write_map/read_write_map.wdl b/centaur/src/main/resources/standardTestCases/read_write_map/read_write_map.wdl index 151637332be..db04711e6c9 100644 --- a/centaur/src/main/resources/standardTestCases/read_write_map/read_write_map.wdl +++ b/centaur/src/main/resources/standardTestCases/read_write_map/read_write_map.wdl @@ -26,30 +26,10 @@ task read_map { } } -task assert_write_map_terminal_newline { - Map[String, String] file_to_name - command <<< - # https://stackoverflow.com/a/25749716/21269164 - file_ends_with_newline() { - [[ $(tail -c1 "$1" | wc -l) -gt 0 ]] - } - - if ! file_ends_with_newline ${write_map(file_to_name)} - then - echo >&2 "Error: write_map() should write a file whose last character is a newline" - exit 1 - fi - >>> - runtime { - docker: "python:3.5.0" - } -} - workflow wf { Map[String, String] map = {"f1": "alice", "f2": "bob", "f3": "chuck"} call write_map {input: file_to_name = map} call read_map - call assert_write_map_terminal_newline {input: file_to_name = map} output { Map[String, Int] out = read_map.out_map String contents = write_map.contents diff --git a/wom/src/main/scala/wom/values/WomMap.scala b/wom/src/main/scala/wom/values/WomMap.scala index bffd26d9e49..5d42b1d3364 100644 --- a/wom/src/main/scala/wom/values/WomMap.scala +++ b/wom/src/main/scala/wom/values/WomMap.scala @@ -69,7 +69,13 @@ final case class WomMap private(womType: WomMapType, value: Map[WomValue, WomVal def tsvSerialize: Try[String] = { (womType.keyType, womType.valueType) match { + case (_: WomPrimitiveType, _: WomPrimitiveType) if value.isEmpty => + // WDL 1.1 spec on `write_map()` https://github.com/openwdl/wdl/blob/main/versions/1.1/SPEC.md#file-write_mapmapstring-string + // Earlier versions of the spec are not as opinionated on `write_map()` behavior. + // "If the Map is empty, an empty file is written." + Success("") case (_: WomPrimitiveType, _: WomPrimitiveType) => + // "All lines are terminated by the newline (\n) character." Success(value.map({case (k, v) => s"${k.valueString}\t${v.valueString}"}).mkString(start="", sep="\n", end="\n")) case _ => Failure(new UnsupportedOperationException("Can only TSV serialize a Map[Primitive, Primitive]")) diff --git a/wom/src/test/scala/wom/types/WomMapTypeSpec.scala b/wom/src/test/scala/wom/types/WomMapTypeSpec.scala index 5bd274b830c..214e150f873 100644 --- a/wom/src/test/scala/wom/types/WomMapTypeSpec.scala +++ b/wom/src/test/scala/wom/types/WomMapTypeSpec.scala @@ -109,4 +109,33 @@ class WomMapTypeSpec extends AnyFlatSpec with CromwellTimeoutSpec with Matchers case _: UnsupportedOperationException => // expected } } + + it should "tsvSerialize non-empty maps correctly and with newlines after every line" in { + stringIntMap.tsvSerialize shouldEqual Success("a\t1\nb\t2\nc\t3\n") + + val intIntMap = WomMap(WomMapType(WomIntegerType, WomIntegerType), Map( + WomInteger(4) -> WomInteger(1), + WomInteger(5) -> WomInteger(2), + WomInteger(6) -> WomInteger(3), + )) + intIntMap.tsvSerialize shouldEqual Success("4\t1\n5\t2\n6\t3\n") + + val stringStringMap = WomMap(WomMapType(WomStringType, WomStringType), Map( + WomString("a") -> WomString("x"), + WomString("b") -> WomString("y"), + WomString("c") -> WomString("z") + )) + stringStringMap.tsvSerialize shouldEqual Success("a\tx\nb\ty\nc\tz\n") + } + + it should "tsvSerialize empty maps to empty Strings" in { + val emptyStringIntMap = WomMap(WomMapType(WomStringType, WomIntegerType), Map.empty) + emptyStringIntMap.tsvSerialize shouldEqual Success("") + + val emptyIntIntMap = WomMap(WomMapType(WomIntegerType, WomIntegerType), Map.empty) + emptyIntIntMap.tsvSerialize shouldEqual Success("") + + val emptyStringStringMap = WomMap(WomMapType(WomStringType, WomStringType), Map.empty) + emptyStringStringMap.tsvSerialize shouldEqual Success("") + } }