Does this still belong under 'Bug fixes' if it's an addition?

IMO it's new functionality not a bug fix

For completeness do you also want a File? yup = ...?

And maybe:

Array[File?] nopes_and_yups = ...
Map[String, File?] nopes_and_yups_map = { "yup": ..., "nope": ... }
Struct foo = object { yup_field: ..., nope_field: ... }

Some all or none of the above additions might or might not work, I think it's valuable even just to know which we're enabling and which are still TODO

We're autogenerating WDL wrappers for all GATK and Picard tools, and one case we 're hoping to get support for is:

output {
        File? nope = ${stringVariableInitializedToNull}
}

Currently this fails (v51 anyway, see https://broadworkbench.atlassian.net/projects/BA/issues/BA-6530). The only workaround I can see at the moment is to use a non-null, made-up fake file name, and hope it doesn't exist. Or maybe there is some other solution?

@cmnbroad I think you'd be better off not wrapping it in ${}? That's interpolating a null into a string, creating a (non-null) empty String which isn't the same as a null string. I suspect there might still be some fixup to make it work but that's probably a necessary first step

@cmnbroad I ran a test with these changes using your WDL from BA-6530. createFile = true and createFile = false jobs now succeed on PAPI but the createFile = false variant subsequently fails with

Bad output 'no_optional_output_task.createdFile': IllegalArgumentException: Cannot coerce the empty String value "" into a File.
	at cromwell.backend.standard.StandardAsyncExecutionActor.$anonfun$handleExecutionSuccess$1(StandardAsyncExecutionActor.scala:916)
	at scala.util.Success.$anonfun$map$1(Try.scala:255)
	at scala.util.Success.map(Try.scala:213)
	at scala.concurrent.Future.$anonfun$map$1(Future.scala:292)
	at scala.concurrent.impl.Promise.liftedTree1$1(Promise.scala:33)
	at scala.concurrent.impl.Promise.$anonfun$transform$1(Promise.scala:33)
	at scala.concurrent.impl.CallbackRunnable.run(Promise.scala:64)
...

which seems like a different / additional problem; I would defer to @cjllanwarne as to whether we should expect this coercion to work. 🙂

Looks like @cjllanwarne's suggestion seems to work with my simple test case. I'll make that change to my WDL generator code and rerun my tests (which take hours to run...) but I suspect that will do it. Thanks to you both for your help!

I guess ideally the evaluateFilesNeededToEvaluate would return FileEvaluations natively rather than having to map in this all-or-nothing way. This way is still infinitely better than the current state of the world but it has a slight potential for risk in edge cases like:

Struct foo {
  File bam
  File? bai
}

Or Pair[File, File?], I suppose

But happy to leave this as a TODO after the release and/or if anyone cares about it

Yes exactly. Given the current state of file evaluation I'm going to make this cautious and only mark files as optional for unambiguous cases.

oh btw call caching appears to be broken with missing optional output files so I'll need to make a ticket for that too 🤕

We should have a case for structs (which I believe are expressed as WomObjects with defined shapes)

in progress

https://youtu.be/qM5W7Xn7FiA?t=56