CWL: Fix step input valueFrom with loadContents: true - #5570
Conversation
valueFrom with loadContents: truevalueFrom with loadContents: true
adamnovak
left a comment
There was a problem hiding this comment.
This looks like it should work.
It seems weird that we only support a file or an array of files, and not, say, an array of arrays of files, but that's indeed what the spec says about loadContents in 1.2:
Only valid when type: File or is an array of items: File.
| Called when loadContents is specified on the input associated with this | ||
| ValueFrom. |
There was a problem hiding this comment.
This isn't actually true, as far as I can tell. It looks like this always gets called, and is itself responsible for checking if loadContents is set.
| Called when loadContents is specified on the input associated with this | |
| ValueFrom. | |
| Responsible for checking loadContents. |
|
The lint tests failed because something is wrong with the type annotations. In a Python virtual environment, after I think the problem is we no longer need to cast |
b5f8f32 to
c8facf4
Compare
Thanks for your review, these mypy issues were fixed. I think instructions could be added to the docs contributing section for running lint tools like mypy, currently the contributing section linked by the "CONTRIBUTING.md" talks about running tests, naming conventions for commit/branch, but not lint unless I missed it. |
This PR implement the resolution of the issue #5568.
Changelog Entry
CWL
loadContents: true(CWL step input errors with valueFrom and loadContents: true #5568)loadContents: true(CWL step input errors with valueFrom and loadContents: true #5568)loadContents: truefor File array (CWL step input errors with valueFrom and loadContents: true #5568)Reviewer Checklist
issues/XXXX-fix-the-thingin the Toil repo, or from an external repo.camelCasethat want to be insnake_case.docs/running/{cliOptions,cwl,wdl}.rstMerger Checklist