Closed illusional closed 3 years ago
Optional nested types, eg: String?[]
If this means "a list of optional strings", then no, aCLImatise doesn't support that. Can you think of an example CLI tool that has such a type? It might be a future direction to move the required/optional into the type object, since it's currently in the Flag
/Positional
Binding the prefix to each element of an array?
As above. Could be supported but would need an example
Tool arguments (maybe this isn't applicable)
What do you mean by this?
Optional nested types, eg: String?[]
I looked through our tool definitions, I don't have an example for this.
Binding the prefix to each element of an array?
Yep, Gatk4ApplyBQSR:
gatk4 ApplyBqsr -I bam -L interval1.bed -L interval2.bed
Tool arguments
In Janis, these are:
--runMode alignReads
) for our StarAlignReads tool, orvalueFrom
, we insist on doing this operation in an argument. But don't need these either.Okay, both of those remaining points could conceivably be parsed from the help text, so I might eventually get around to them. But since I don't support that yet you don't have to worry about converting them for now.
Hi @TMiguelT, it looks like the CI is failing with an isort
error. It's trying to move the deprecated
import, though when I ran isort
locally this is where it placed it -> I'm running VERSION 5.7.0
of isort
if that helps. Not sure what to do because moving the import manually will cause my precommit hook to fail.
Oh yes this is a strange and annoying error. I've been chucking the third party libs in the .isort.cfg
which seems to sort it.
Hey @TMiguelT, tests are passing now. Are there any other steps to follow?
Nope, just my (upcoming) code review, and it might be worth eyeballing the outputs to see if they make sense (and incorporating anything you note into the validation function)
Thanks for the feedback @TMiguelT, I've addressed your feedback by importing janis_core at the top (rather than only when initialised), it provides the benefit of type annotations at the expense of some extra loading time of janis_core (but that shouldn't impact this).
At some point I'll work out a system that only conditionally imports the generator modules, meaning that users who don't want Janis won't have to install or load the Janis module. But you shouldn't have to worry about that. Also fwiw I believe you could have done the type annotations using strings to prevent the need for a global import.
Anyway, it's fine as it is.
Tests all pass :)
Great!
Hey @illusional, I've had this merged into the Base Camp for a while, but I note that it's generating the Python class all on one line, e.g. https://aclimatise.github.io/BaseCamp//packages/samtools/1.11/samtools_dict/samtools_dict.py. Is there a simple fix for this?
Thanks for question @TMiguelT! If the black formatted is installed, it will automatically format :)
As in I just have to passively have it installed when the Janis convert is run?
Yep!
A few questions still to answer:
String?[]