Map literals: improve type inference and error messages

[nh13]

The struct has type File for member some_file, and task some_task has an output member some_file that has type File, so why is miniwdl check failing?

(test.wdl Ln 14 Col 26) Expected Array[File] instead of File
                "some_file": some_task.some_file,
                             ^^^^^^^^^^^^^^^^^^^
* Hint: for compatibility with older existing WDL code, try setting --no-quant-check to relax quantifier validation rules.

version 1.0
struct FooStruct {
    Array[File] list_of_files
    File some_file
}

workflow some_workflow {
    input {
        String file_name
    }
    output {
        FooStruct out = {
            "list_of_files": some_task.list_of_files,
            "some_file": some_task.some_file,
        }
    }
    call some_task  {
        input:
            file_name=file_name
    }
}

task some_task {
    input {
        String file_name
    }
    output {
      Array[File] list_of_files = glob("~{file_name}*.txt")
      File some_file = "~{file_name}.txt"
    }
    command <<<
        echo Hi > ~{file_name}.txt
    >>>
}

[mlin]

Hi @nh13, thanks for reporting -- this is a pretty gnarly subtlety arising from how WDL structs are currently (prior to openwdl/wdl#297) initialized from Map and object literals. In the WDL type system your literal { "list_of_files": some_task.list_of_files, "some_file": some_task.some_file, } is actually a Map[K,V] prior to its intended coercion to FooStruct. As a Map[K,V], miniwdl therefore wants all its right-hand-side values to have the same type, which is the source of the error.

1. You can use the object literal syntax to initialize FooStruct out = object { list_of_files: some_task.list_of_files, some_file: some_task.some_file } as this doesn't have the aforementioned constraint on the value types.
2. miniwdl might be more anal about requiring the map key/value types to unify than the Cromwell typechecker (which dxWDL shares); is this what you've found?
3. Either way miniwdl should provide a more specific error message for failures to unify map literal types; I had to look carefully at the stack trace to see exactly where it was coming from.

[mlin]

Also, #255 shows that the type unification logic for map literal is lacking in sophistication (fails to unify File and File? to File?) -- cf. the logic used for if/then/else

miniwdl/WDL/Expr.py

Lines 690 to 694 in d250427

	# Unify consequent & alternative types. Subtleties:
	# 1. If either is optional, unify to optional
	# 2. If one is Int and the other is Float, unify to Float
	# 3. If one is a nonempty array and the other is a possibly empty
	# array, unify to possibly empty array

[nh13]

I’ll use the object literal constructor going forward but I’ll leave it open in case someone wants to implement it.

[Melkaz]

Hello !
I'm not sure if it's supposed to be fixed, but should we continue to use object to initialize ?

For example, this example fails with version 1.4.1:

version 1.0

struct TestStruct {
    String aString
    Boolean aBoolean
}

task Task {
    input {
        String myString = "Hello Mike !"
    }
 
    command {
        echo ~{myString}
    }

    output {
        TestStruct myTest = { "aString": "1", "aBoolean": true }
    }
}

Thanks :)

[ynedelec]

@mlin I'm sorry to bother you but could you please re-open that issue ?
Thanks

[mlin]

@Melkaz try inserting the object keyword TestStruct myTest = object { "aString": "1", "aBoolean": true }

Without the keyword, the structure in the braces is understood as a Map with homogeneous value types (implicitly coercing the boolean to a string, a one-way door). Whereas object is a bag of mixed value types.

This is a significant pitfall in the language obviously, I'll think about ways to provide a better error message in this case.

@ynedelec were you just pinging o/b/o @Melkaz or did you have a separate problem? thanks

[ynedelec]

Thanks Mike.
Sorry, Melkaz is my other GitHub account that I switched on by mistake.

I'm good for using object, but is this behavior a pitfall of the langage or miniWDL ?
Do you know if Cromwell requires object as well ?

I'm advocating for changing to WDL and I must pay attention to those little quirks when I'll train people in order to simplify their work.

Thanks :)

[mlin]

@ynedelec I think the confusion between Map and object literals is inherent in WDL but I'm not sure exactly how Cromwell handles all these cases. The key fact relevant here is that a Map has homogeneous value types (and may have non-string keys).

WDL 1.1 has a new struct initialization syntax, but it's essentially the same old object initializer except you replace object with the name of the struct.