-
Notifications
You must be signed in to change notification settings - Fork 750
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 pb deepvariant #7363
base: master
Are you sure you want to change the base?
Update pb deepvariant #7363
Conversation
@@ -44,9 +50,15 @@ nextflow_process { | |||
|
|||
test("human - bam - intervals") { | |||
|
|||
config './nextflow.config' |
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.
If this is now added to all tests we can also move it to the top of the file avoiding too much code duplication :)
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 good idea - we can do that! This seemed to be the pattern used in other module tests, but happy to adjust!
tuple val(ref_meta), path(fasta) | ||
path model_file |
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.
Can you also include a test that uses this model file?
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 good idea - @gburnett-nvidia and I will find a model file and add a test. Will have to also find a way to host the model file... somewhere.
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.
the model file can go into nf-core/test-datasets
tuple val(meta), path("*.vcf"), optional: true, emit: vcf | ||
tuple val(meta), path("*.g.vcf"), optional: true, emit: gvcf | ||
path "versions.yml", emit: versions |
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.
Can you align these nicely? (as it was before)
tuple val(meta), path("*.vcf"), optional: true, emit: vcf | ||
tuple val(meta), path("*.g.vcf"), optional: true, emit: gvcf |
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.
Is there a possibility to return gzipped vcf files as standard?
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.
Right now there is no way for Parabricks directly to return gzipped vcf files. It would have to be added as an additional step.
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.
Maye you can add bgzip to the parabricks docker/singularity container? Then we could easily do it in the script section of the module. That would be very helpful!
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.
PR checklist
Closes #XXX
versions.yml
file.label
nf-core modules test <MODULE> --profile docker
nf-core modules test <MODULE> --profile singularity
nf-core modules test <MODULE> --profile conda
nf-core subworkflows test <SUBWORKFLOW> --profile docker
nf-core subworkflows test <SUBWORKFLOW> --profile singularity
nf-core subworkflows test <SUBWORKFLOW> --profile conda