adjust String/File/Directory coercions (#669)

chanzuckerberg · Feb 19, 2024 · 4a6157f · 4a6157f
1 parent c5bb92b
commit 4a6157f
Show file tree

Hide file tree

Showing 4 changed files with 63 additions and 11 deletions.
diff --git a/WDL/Lint.py b/WDL/Lint.py
@@ -287,8 +287,11 @@ def expr(self, obj: Expr.Base) -> Any:
                 F = getattr(
                     StdLib.TaskOutputs(_find_doc(obj).effective_wdl_version), obj.function_name
                 )
-                if isinstance(F, StdLib.StaticFunction) and obj.function_name != "basename":
-                    # ok for basename to take either String or File
+                if isinstance(F, StdLib.StaticFunction) and obj.function_name not in (
+                    "basename",  # ok to take either String or File
+                    "write_lines",  # clear intent
+                    "write_tsv",  # clear intent
+                ):
                     for i in range(min(len(F.argument_types), len(obj.arguments))):
                         F_i = F.argument_types[i]
                         arg_i = obj.arguments[i]

diff --git a/WDL/Type.py b/WDL/Type.py
@@ -575,16 +575,10 @@ def unify(types: List[Base], check_quant: bool = True, force_string: bool = Fals
         # Int/Float, String/File
         if isinstance(t, Int) and isinstance(t2, Float):
             t = Float()
-        if isinstance(t, String) and isinstance(t2, File):
-            t = File()
-        if isinstance(t, String) and isinstance(t2, Directory):
-            t = Directory()
 
         # String
         if (
             isinstance(t2, String)
-            and not isinstance(t2, (File, Directory))
-            and not isinstance(t, (File, Directory))
             and (not check_quant or not isinstance(t, Array))
             and (not isinstance(t, (Pair, Map)))
         ):

diff --git a/test_corpi/contrived/issue669.wdl b/test_corpi/contrived/issue669.wdl
@@ -0,0 +1,53 @@
+version 1.0
+
+task TestTask {
+
+    input {
+        Array[String] string_list 
+        Array[Boolean] bool_list_1 
+        Array[Boolean] bool_list_2 
+        Array[Boolean] bool_list_3 
+        Array[Float] float_list 
+        Array[File] file_list
+    }
+
+    command <<<
+      # Passing Tests:
+
+      # Trivial Tests:
+      cat ~{write_tsv(transpose([string_list]))}
+      cat ~{write_tsv(transpose([bool_list_1]))}
+      cat ~{write_tsv(transpose([float_list]))}
+      cat ~{write_tsv(transpose([file_list]))}
+      cat ~{write_tsv(transpose([bool_list_1, bool_list_2, bool_list_3]))}
+
+      # Basic Type coercion tests:
+      cat ~{write_tsv(transpose([string_list, file_list]))}
+      cat ~{write_tsv(transpose([string_list, file_list, bool_list_1]))}
+
+      cat ~{write_tsv(transpose([string_list, bool_list_1, bool_list_2, bool_list_3]))}
+      cat ~{write_tsv(transpose([string_list, bool_list_1, bool_list_2, bool_list_3, float_list]))}
+
+      # Special test case:
+      cat ~{write_tsv(transpose([string_list, file_list, float_list]))}
+
+      # Failing Tests:
+
+      # NOTE: This is just a re-ordering of the special test case, which passes.
+      cat ~{write_tsv(transpose([string_list, float_list, file_list]))}
+
+      # Original code from WDL:
+      # NOTE: This code runs correctly on Cromwell (V87-e3a923f):
+      cat ~{write_tsv(transpose([string_list, bool_list_1, bool_list_2, bool_list_3, float_list, file_list]))}
+    >>>
+
+    output {
+      Array[String] stdout = read_lines(stdout())
+    }
+
+    #########################
+    runtime {
+        cpu:    1 
+        docker: "ubuntu:22.04"
+    }
+}
diff --git a/tests/test_3corpi.py b/tests/test_3corpi.py
@@ -188,7 +188,7 @@ class GATK_five_dollar(unittest.TestCase):
     ["test_corpi/gatk-workflows/gatk4-germline-snps-indels/**"],
     expected_lint={
         "UnusedDeclaration": 3,
-        "StringCoercion": 20,
+        "StringCoercion": 15,
         "FileCoercion": 3,
         "UnknownRuntimeKey": 1,
         "MissingVersion": 4,
@@ -306,7 +306,7 @@ class ViralNGS(unittest.TestCase):
     ["test_corpi/ENCODE-DCC/chip-seq-pipeline2/**"],
     expected_lint={
         "StringCoercion": 208,
-        "FileCoercion": 154,
+        "FileCoercion": 170,
         "NameCollision": 16,
         "OptionalCoercion": 64,
         "MixedIndentation": 32,
@@ -326,7 +326,7 @@ class ENCODE_ChIPseq(unittest.TestCase):
         "OptionalCoercion": 1020,
         "UnusedCall": 45,
         "StringCoercion": 30,
-        "FileCoercion": 71,
+        "FileCoercion": 236,
         "MissingVersion": 29,
     },
     check_quant=False,
@@ -471,6 +471,7 @@ class BioWDLTasks(unittest.TestCase):
 @wdl_corpus(
     ["test_corpi/biowdl/aligning/**"],
     expected_lint={
+        "FileCoercion": 1,
         "OptionalCoercion": 11,
         "UnusedDeclaration": 12,
         "NonemptyCoercion": 1,
@@ -487,6 +488,7 @@ class BioWDLAligning(unittest.TestCase):
 @wdl_corpus(
     ["test_corpi/biowdl/expression-quantification/**"],
     expected_lint={
+        "FileCoercion": 1,
         "OptionalCoercion": 11,
         "UnusedDeclaration": 12,
         "NonemptyCoercion": 3,