-
Notifications
You must be signed in to change notification settings - Fork 712
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
update bcftools concat #3636
update bcftools concat #3636
Conversation
|
||
output: | ||
tuple val(meta), path("*.gz"), emit: vcf | ||
path "versions.yml" , emit: versions | ||
tuple val(meta), path("*.${extension}") , emit: vcf |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be better to rename the output given the more possibilities for output type?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, but I would maybe do this change for all other modules in another PR (since all other modules where this extension calculation happen also have vcf
as output name)
|
||
BCFTOOLS_CONCAT ( input ) | ||
} | ||
bed = file(params.test_data['homo_sapiens']['genome']['genome_bed'], checkIfExists: true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm assuming you meant to add the bed int the test- but I guess there should be a test both with and without a bed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll add it, thanks!
|
||
when: | ||
task.ext.when == null || task.ext.when | ||
|
||
script: | ||
def args = task.ext.args ?: '' | ||
prefix = task.ext.prefix ?: "${meta.id}" | ||
def args = task.ext.args ?: '--output-type z' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will set the output type in the absence of args, right? Would it not be better to append the output type if it's not present in args (allowing for other things to be set there)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, although this maybe is a bit too much overhead. It's a best practice anyway to specify it yourself IMO
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In any case, it doesn't make sense to condition this on the setting of task.ext.args in general.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I should remove the default arg? :p
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was fine with the setting of the default, I just thought it was a bit fragile to assume this was the only param.
e.g. if in future you had task.ext.args equal to:
--another-param blah
Then you would still want to append your default, even though task.ext.args is already set.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes I get it :). I think I'll leave it with the tool defaults for now though. it's easy to change the output extension now
Co-authored-by: Jonathan Manning <[email protected]>
@pinin4fjords tests seem to be all good now :p |
The merge conflict has also been resolved now - LGTM further. |
PR checklist
Closes #XXX
versions.yml
file.label
PROFILE=docker pytest --tag <MODULE> --symlink --keep-workflow-wd --git-aware
PROFILE=singularity pytest --tag <MODULE> --symlink --keep-workflow-wd --git-aware
PROFILE=conda pytest --tag <MODULE> --symlink --keep-workflow-wd --git-aware