-
Notifications
You must be signed in to change notification settings - Fork 713
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
Changes from 18 commits
8df2221
3780c6c
4a4979c
5533a7e
0b8330f
f772c25
d84dc41
f980e00
e82b71c
289314b
eb7ca02
a228da4
0ea95a6
4faef2f
0320ae8
48db0ba
955a5ff
014790d
c0c8ed4
6857f7e
c4b3605
ba8408d
6ab1d32
969086b
9b31c8f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,23 +8,32 @@ process BCFTOOLS_CONCAT { | |
'biocontainers/bcftools:1.17--haef29d1_0' }" | ||
|
||
input: | ||
tuple val(meta), path(vcfs), path(tbi) | ||
tuple val(meta), path(vcfs), path(tbis) | ||
path(bed) | ||
|
||
output: | ||
tuple val(meta), path("*.gz"), emit: vcf | ||
path "versions.yml" , emit: versions | ||
tuple val(meta), path("*.${extension}") , emit: vcf | ||
path "versions.yml" , emit: versions | ||
|
||
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 commentThe 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 commentThe 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 commentThe 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 commentThe 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 commentThe 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:
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 commentThe 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 |
||
def prefix = task.ext.prefix ?: "${meta.id}" | ||
def regions = bed ? "--regions-file ${bed} --allow-overlaps" : '' // --allow-overlaps is required for bcftools concat to work with bed files | ||
|
||
extension = args.contains("--output-type b") || args.contains("-Ob") ? "bcf.gz" : | ||
args.contains("--output-type u") || args.contains("-Ou") ? "bcf" : | ||
args.contains("--output-type z") || args.contains("-Oz") ? "vcf.gz" : | ||
args.contains("--output-type v") || args.contains("-Ov") ? "vcf" : | ||
"vcf" | ||
""" | ||
bcftools concat \\ | ||
--output ${prefix}.vcf.gz \\ | ||
$args \\ | ||
--threads $task.cpus \\ | ||
--output ${prefix}.${extension} \\ | ||
${args} \\ | ||
${regions} \\ | ||
--threads ${task.cpus} \\ | ||
${vcfs} | ||
|
||
cat <<-END_VERSIONS > versions.yml | ||
|
@@ -34,9 +43,15 @@ process BCFTOOLS_CONCAT { | |
""" | ||
|
||
stub: | ||
prefix = task.ext.prefix ?: "${meta.id}" | ||
def prefix = task.ext.prefix ?: "${meta.id}" | ||
|
||
extension = args.contains("--output-type b") || args.contains("-Ob") ? "bcf.gz" : | ||
args.contains("--output-type u") || args.contains("-Ou") ? "bcf" : | ||
args.contains("--output-type z") || args.contains("-Oz") ? "vcf.gz" : | ||
args.contains("--output-type v") || args.contains("-Ov") ? "vcf" : | ||
"vcf" | ||
""" | ||
touch ${prefix}.vcf.gz | ||
touch ${prefix}.${extension} | ||
|
||
cat <<-END_VERSIONS > versions.yml | ||
"${task.process}": | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,23 +6,19 @@ include { BCFTOOLS_CONCAT } from '../../../../../modules/nf-core/bcftools/concat | |
|
||
workflow test_bcftools_concat_tbi { | ||
|
||
input = [ [ id:'test3' ], // meta map | ||
[ file(params.test_data['sarscov2']['illumina']['test_vcf_gz'], checkIfExists: true), | ||
file(params.test_data['sarscov2']['illumina']['test2_vcf_gz'], checkIfExists: true) ], | ||
[ file(params.test_data['sarscov2']['illumina']['test_vcf_gz_tbi'], checkIfExists: true), | ||
file(params.test_data['sarscov2']['illumina']['test2_vcf_gz_tbi'], checkIfExists: true) ] | ||
] | ||
input = [ | ||
[ id:'test' ], // meta map | ||
[ | ||
file(params.test_data['homo_sapiens']['illumina']['test_haplotc_cnn_vcf_gz'], checkIfExists: true), | ||
file(params.test_data['homo_sapiens']['illumina']['test_genome_vcf_gz'], checkIfExists: true) | ||
], | ||
[ | ||
file(params.test_data['homo_sapiens']['illumina']['test_genome_vcf_gz_tbi'], checkIfExists: true), | ||
file(params.test_data['homo_sapiens']['illumina']['test_haplotc_cnn_vcf_gz_tbi'], checkIfExists: true) | ||
] | ||
] | ||
|
||
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 commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. I'll add it, thanks! |
||
|
||
workflow test_bcftools_concat_no_tbi { | ||
|
||
input = [ [ id:'test3' ], // meta map | ||
[ file(params.test_data['sarscov2']['illumina']['test_vcf_gz'], checkIfExists: true), | ||
file(params.test_data['sarscov2']['illumina']['test2_vcf_gz'], checkIfExists: true) ], | ||
[] | ||
] | ||
|
||
BCFTOOLS_CONCAT ( input ) | ||
} | ||
BCFTOOLS_CONCAT ( input, [] ) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,25 +1,9 @@ | ||
- name: bcftools concat test_bcftools_concat_tbi | ||
command: nextflow run ./tests/modules/nf-core/bcftools/concat -entry test_bcftools_concat_tbi -c ./tests/config/nextflow.config -c ./tests/modules/nf-core/bcftools/concat/nextflow.config | ||
command: nextflow run ./tests/modules/nf-core/bcftools/concat -entry test_bcftools_concat_tbi -c ./tests/config/nextflow.config -c ./tests/modules/nf-core/bcftools/concat/nextflow.config | ||
tags: | ||
- bcftools | ||
- bcftools/concat | ||
files: | ||
- path: output/bcftools/test3.vcf.gz | ||
md5sum: 18c1612343f5e8a219ee6476a870a674 | ||
|
||
- name: bcftools concat test_bcftools_concat_no_tbi | ||
command: nextflow run ./tests/modules/nf-core/bcftools/concat -entry test_bcftools_concat_no_tbi -c ./tests/config/nextflow.config -c ./tests/modules/nf-core/bcftools/concat/nextflow.config | ||
tags: | ||
- bcftools | ||
- bcftools/concat | ||
files: | ||
- path: output/bcftools/test3.vcf.gz | ||
md5sum: 18c1612343f5e8a219ee6476a870a674 | ||
|
||
- name: bcftools concat test_bcftools_concat_tbi_stub | ||
command: nextflow run ./tests/modules/nf-core/bcftools/concat -entry test_bcftools_concat_tbi -c ./tests/config/nextflow.config -c ./tests/modules/nf-core/bcftools/concat/nextflow.config -stub | ||
tags: | ||
- bcftools | ||
- bcftools/concat | ||
files: | ||
- path: output/bcftools/test3.vcf.gz | ||
- path: output/bcftools/test.vcf.gz | ||
md5sum: 4bcd0afd89f56c5d433f6b6abc44d0a6 | ||
- path: output/bcftools/versions.yml |
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)