Fix many issues with examples #669
Conversation
spec for map types
|
@stxue1 @adamnovak would also appreciate your reviews |
| @@ -534,7 +535,12 @@ The following primitive types exist in WDL: | |||
| * A `File` represents a file (or file-like object). | |||
| * A `File` declaration can have a string value indicating a relative or absolute path on the local file system. | |||
| * Within a WDL file, literal values for files may only be local (relative or absolute) paths. | |||
| * The path assigned to a `File` is not required to be valid unless and until it is accessed. | |||
| * To read from a file, it must exist and be assigned appropriate permissions. | |||
| * To write to a file, the parent directory must be assigned appropriate permissions. | |||
There was a problem hiding this comment.
WDL File objects are snapshots, right? Any WDL function that writes a File returns the written File and doesn't take a File as input to overwrite.
Or does this mean within a task, if you substitute a File's name into a command that tries to overwrite it, the parent directory that the File initially came from should be writable?
There was a problem hiding this comment.
It means the directory in the execution environment where the file is to be written must have writable permissions.
There was a problem hiding this comment.
For example, if I define File outfile = "outfile" and then in the command do printf "hello" > ~{outfile}, the directory where the execution engine resolves outfile (presumably, the working directory, but it doesn't have to be) must be writable.
There was a problem hiding this comment.
There is not really any reason to use a File for this purpose, and (obviously) it adds a lot of confusion. Hence the impetus to disallow relative paths.
| File infile | ||
| } | ||
|
|
||
| String path1 = "~{infile}" |
There was a problem hiding this comment.
This makes the test now depend on file to string conversion as well as string to file conversion. But I don't think the test framework lets you set up actual file fixtures to make this work a different way.
Or could we just use hello.txt, since it's available to use in the input JSON?
| "import_structs.bill": 175000 | ||
| "import_structs.bill": 175000.0 |
There was a problem hiding this comment.
At least under the interpretation of JSON that JSON Schema uses, these are the JSON value.
Are we expecting the runner to produce one serialization here and not the other? Or @stxue1 are you doing something that needs to key on actually having a decimal point?
There was a problem hiding this comment.
JSON doesn't make a distinction between int and float. The test framework should compare these values as equal.
There was a problem hiding this comment.
Are we expecting the runner to produce one serialization here and not the other? Or @stxue1 are you doing something that needs to key on actually having a decimal point?
I currently do try to key on the decimal point to test for floats, but since the JSON schema doesn't actually support a distinction for floats and ints, there isn't much reason to actually key on decimal points, so I'll remove the check from our test suite.
SPEC.md
Outdated
| hisat2 \ | ||
| -p ~{threads} \ | ||
| ~{if defined(max_reads) then "-u ~{select_first([max_reads])}" else ""} \ | ||
| -x index/~{index_id} \ | ||
| -x index/grch38/genome \ |
There was a problem hiding this comment.
This looks like we're hardcoding for the name of the genome that actually happens to be in this file; we also no longer use index_id at all.
Maybe we meant to generate this string from the filename?
| -x index/grch38/genome \ | |
| -x index/"$(echo ~{index_id} | tr '_' '/')" \ |
There was a problem hiding this comment.
I'm guessing the tgz file contains the reference nested two levels deep. Rather than hard-code the path, I used the --strip-components 2 argument to tar.
| Array[Array[Int]] out = transpose(input_array) | ||
| Array[Array[Int]] expected = expected_output_array |
There was a problem hiding this comment.
The example output section is missing the 2 new outputs.
The example output json below is currently defined as:
{
"test_transpose.is_true": true
}and should become
{
"test_transpose.expected": [[0, 3], [1, 4], [2, 5]],
"test_transpose.out": [[0, 3], [1, 4], [2, 5]],
"test_transpose.is_true": true
}|
|
||
| output { | ||
| Boolean paths_equal = path2 == path3 | ||
| Boolean paths_equal = infile == path3 |
There was a problem hiding this comment.
path3 is undefined, I think it should be replaced with path2?
Closes #653
Closes #654
Closes #661
Closes #662
Closes #663
Closes #664
Closes #665
Closes #666
Closes #667
Closes #668
Checklist