Modules required for DSL 2 implementation of pipeline about viralrecon HOT 17 CLOSED

What @heuermh said and https://github.com/nf-core/modules for good measure.

from viralrecon.

Goodness me, that's a bigger list than I was expecting!! 😅

from viralrecon.

@cjfields Yes, there is plenty of discussion in the modules channel on Slack (https://nfcore.slack.com/archives/CJRH30T6V) and I believe the https://github.com/nf-core/rnaseq workflow is leading the way

from viralrecon.

@drpatelh you can assign ivar to me, I have then written and just adding some tests.

I fixed the ivar bioconda recipe to depend on Samtools, so that the container and conda environment will contain samtools. This address the issue where, in some case, ivar requires samtools (e.g., samtools mpileup | ivar consensus)

Update the tag to _1: ivar:1.3--h089eab3_0 -> ivar:1.3--h089eab3_1 to use the updated version of the bioconda recipe.

from viralrecon.

@drpatelh can you send me the issue you are having with spades on bioconda? i can work with it.

from viralrecon.

@JoseEspinosa yes, my bad, I still need to do a little work on that process but I have moved it to the "not fit for nf-core/modules" section at the bottom. It is commented out in the main workflow because I added some fresh sub-workflows for each variant caller at the end of last week, and I need to sort out all of the channels to get it working again 🙂

Currently, the reason we can't use a simple collect there is because I have used exactly the same naming convention for each of the callers for simplicity. This means if we stage them all together then Nextflow will barf. This is why they are being staged in their own directories at the moment. This one may have to stay as a local module but you could maybe add a generic one to nf-core/modules that just takes a list of tuple val(meta), path(vcfs). It would be a good start anyway and we almost certainly expect these modules to be updated. Adding the tests and docs is always the bit that takes the longest 😅

from viralrecon.

Rather than add samtools as a separate step (I am thinking of minimising having to spin up containers), would it better off to add tee to the step and capture the output --- perhaps optionally:

Great idea! Sounds like a good compromise 👍🏽 I will just pass the same mpileup generating arguments I am using for VarScan here too.

from viralrecon.

Awesome work guys! Especially @JoseEspinosa 😎 Going to close this issue since we are pretty much done. Thank you!

from viralrecon.

Once the pattern is in place, I should be able to handle the seqwish/vg ones

from viralrecon.

Following on what @heuermh said: is there a general 'nf-core'-y template (a standard way) that DSL2 modules are being set up? This would be of general use to other Nextflow and nf-core projects

from viralrecon.

Modules ready to push to nf-core/modules

• abacas.nf (assigned @JoseEspinosa #431)
• artic_guppyplex.nf (assigned @JoseEspinosa #455)
• artic_minion.nf (assigned @JoseEspinosa)#486
• bandage.nf (assigned @heuermh #146)
• bcftools_bgzip.nf (assigned @JoseEspinosa #134)
• bcftools_consensus.nf (assigned @JoseEspinosa #134)
• bcftools_filter.nf (assigned @JoseEspinosa #134)
• bcftools_merge.nf (assigned @JoseEspinosa #168)
• [bcftools_mpileup.nf](assigned @JoseEspinosa #247)
• bcftools_stats.nf (assigned @JoseEspinosa #134)
• bcftools_tabix.nf (assigned @JoseEspinosa #134)
• bedtools_getfasta.nf (assigned @JoseEspinosa #161)
• bedtools_maskfasta.nf (assigned @JoseEspinosa #153)
• bedtools_merge.nf (assigned @JoseEspinosa #79)
• blast_blastn.nf (assigned @JoseEspinosa #160)
• blast_makeblastdb.nf (assigned @JoseEspinosa #159)
• bowtie2_align.nf (assigned @JoseEspinosa #127)
• bowtie2_build.nf (assigned @JoseEspinosa #127)
• cat_fastq.nf (assigned @JoseEspinosa #167)
• fastp.nf (assigned @KevinMenden #131)
• gunzip.nf (assigned @JoseEspinosa #157)
• ivar_consensus.nf (assigned @andersgs #174)
• ivar_trim.nf (assigned @andersgs #170)
• ivar_variants.nf (assigned @andersgs #172)
• kraken2_run.nf(assigned @JoseEspinosa #266)
• minia.nf (assigned to @KevinMenden #267 )
• mosdepth.nf (assigned @JoseEspinosa #171)
• nanoplot.nf (added #364)
• nextclade.nf (added #484)
• pangolin.nf (assigned @KevinMenden #124)
• picard_collectwgsmetrics.nf (see nf-core/modules#264)
• plasmidid.nf(assigned @JoseEspinosa #410)
• pycoqc.nf(assigned @JoseEspinosa #424)
• quast.nf (assigned @KevinMenden #130)
• samtools_faidx.nf (#218)
• samtools_mpileup.nf (assigned @JoseEspinosa #126)
• samtools_view.nf (assigned @JoseEspinosa #126)
• spades.nf(assigned @JoseEspinosa #277)
• unicycler.nf(assigned @JoseEspinosa #158)
• untar.nf (assigned @JoseEspinosa #307)

Modules with customisation that we may not be able to push to nf-core/modules

• bcftools_isec.nf - VCFs from different callers are named the same and have to be staged in explicit folders within the process
• bedtools_genomecov.nf - Only supports BAM input currently
• cutadapt.nf - Have to stage a separate adapters.fa for primer trimming
• kraken2_build.nf - Unfinished - requires more thought!
• snpeff_ann.nf - Requires custom database and staging options
• snpeff_build.nf - Only supports custom database build for the pipeline
• snpsift_extractfields.nf - Using custom options, attempt to make the module more generic

from viralrecon.

Hi @drpatelh I have a couple of questions regarding the implementation of the bcftools_isec module, first I am not sure whether it will be used since it is now commented.
Then, I saw that each vcf file (and corresponding tbi index) is provided as a separate item of the declaration of the input channel. Wouldn't it be more convenient to declare a single path and collect all the files before inputting them in the module. This way the declaration of the module input would be more generic.

from viralrecon.

Amazing! Thanks @andersgs 😎 I will wait for you to push the modules here before I install and use them in viralrecon. It would be great if ivar variants and ivar consensus are able to support both BAM and mpileup input. We have had questions posed about variants being called where we have had to ask users to go back to the mpileup files. That's one of the reasons I have an explicit step for mpileup generation, and the other one is that the mpileup file can be generated with exactly the same parameters and passed to VarScan2 too. We are using BAM input and pipes for BCFTools via bcftools mpileup. Let's discuss on your PRs 👍🏽

from viralrecon.

Rather than add samtools as a separate step (I am thinking of minimising having to spin up containers), would it better off to add tee to the step and capture the output --- perhaps optionally:

samtools mpileup | tee pileup.txt | ivar consensus

I agree that we should be moving the bcftools mpileup (since samtools mpileup is being deprecated) but the docs for ivar are clear on depending on samtools mpileup, I haven't chased up with the ivar team on this.

from viralrecon.

I am assuming the Andersen lab have been ridiculously over-run like all of us 😓. We added a MultiQC module for iVar last year and a script to convert the tabular output to VCF. Still waiting for the latter to be added officially too.

andersen-lab/ivar#17

from viralrecon.

@drpatelh bandage.nf looks reasonable to me, what do you need to push to nf-core/modules? I can pick that up if you'd like. Assign minimap2 to me as well.

from viralrecon.

That would be great @heuermh. I assigned it to you in the issue above 👍🏽 I haven't written a module for minia (or any of the other variant graph modules) yet but feel free to assign yourself / add any of the others above too 🙂 I have been quite pre-occupied with overhauling the CI tests we are using on nf-core/modules but hoping to come back to a sprint here soon.

from viralrecon.