Skip to content

Cleanup command line builder - #5

Merged
illusional merged 3 commits into
masterfrom
cleanup-commandbuilder
Jul 13, 2020
Merged

Cleanup command line builder#5
illusional merged 3 commits into
masterfrom
cleanup-commandbuilder

Conversation

@illusional

Copy link
Copy Markdown
Member

I've never really been happy with the command line builder - this has sort of led me to half-implement building the command line with wdlgen, and half in an external project.

I guess it depends what you want from this project, but I don't believe the core should be taking care of that for you - I believe it should be a more faithful representation of WDL.

For this reason, in v0.3.0 I've removed the option to provide components to the Task.Command.CommandInput (which would construct the workflow for you). This behaviour will still exist temporarily in in Task.Command.CommandInput.from_fields, but with a number of PRs coming to cleanup these string interpolation options and provide a quoting mechanic, I don't think we need to do this.

@illusional

illusional commented May 7, 2020

Copy link
Copy Markdown
MemberAuthor

Hey @TMiguelT, just FYI for this PR and v0.3.0 release.

Specifically removing the interpolation options for these reasons:

I'll attach the code I now use for quoting the command line. I use the updated sep function (because I implemented in Cromwell and miniWDL), and once the quoting functions are implemented I'll merge those in. For transparency, I'm not attempting to generate especially readable code, purely functional :

https://github.com/PMCC-BioinformaticsCore/janis-core/blob/cb1e87a3440a8920afe8d21f840b1f6bfaf39ba7/janis_core/translations/wdl.py#L976-L1066

Edit: Fixed a few things with my binding (optional arrays, arrays of optional types, prefix generation for arrays)

@illusional

Copy link
Copy Markdown
MemberAuthor

The OpenWDL PR has been merged, including implementations in Cromwell and MiniWDL - so this will merge.
I've added a change to remove the command options so they should work forwards. If you need the current existing functionality, you'll need to illusional.wdlgen to <0.3.0, eg:

https://github.com/PMCC-BioinformaticsCore/janis-core/blob/93f6e1aaba4d4c221706f2ad0aa73b6d884ebb46/setup.py#L55

@illusional
illusional merged commit a9b029f into masterJul 13, 2020
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@illusional