Add BISCUIT #186
Closed
Add BISCUIT #186
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ate multiqc accordingly, added picard CollectInsertSizeMetrics and CollectGcBiasMetrics for all aligners, added Samtools sort for bismark aligner. The fasta is assumed to contain the assembly name, not just genome.fa. DOCKERFILE, enviroment.yml, parameters.settings.json, bin/scrape_software_versions.py, conf/base.config, README.md were changed accordingly. Bin/biscuit_QC.sh, bin/setup.sh were added for biscuit_QC step
… Update enviroment.yml to newer version. Change main.nf slightly, on definision of publish output directory
…thylseq. adde biscuit to CL.
Co-Authored-By: Patrick Hüther <patrick.huether@gmail.com>
Co-Authored-By: Patrick Hüther <patrick.huether@gmail.com>
Co-Authored-By: Patrick Hüther <patrick.huether@gmail.com>
Co-Authored-By: Patrick Hüther <patrick.huether@gmail.com>
Co-Authored-By: Patrick Hüther <patrick.huether@gmail.com>
Co-Authored-By: Patrick Hüther <patrick.huether@gmail.com>
Co-Authored-By: Patrick Hüther <patrick.huether@gmail.com>
Co-Authored-By: Patrick Hüther <patrick.huether@gmail.com>
…d a process to build biscuitQC_assets, and running biscuit_QC.sh and built_biscuit_QC_assets.pl from within bioconda recipe
…bsnp.hdr to assets dir
…fix problems in main
…d qualimap), fix skip_deduplication for biscuit, change environment.yml file
ewels
reviewed
Feb 22, 2021
ewels
reviewed
Feb 22, 2021
phue
requested changes
Feb 22, 2021
nextflow.config
Outdated
| skip_deduplication = false | ||
| non_directional = false | ||
| nondirectional_library = false |
There was a problem hiding this comment.
the bismark workflow also has params.non_directional, could reuse that for BISCUIT aswell
There was a problem hiding this comment.
That's what I did in the beginning, but then I had a problem witth the nextflow_schema.json file, so I added this and min_coverage (instead of min_depth) for the BISCUIT aligner
There was a problem hiding this comment.
What was the problem? That the parameter can't be in the schema twice and so you have to choose between the Bismark and Biscuit groups?
If so then in other cases I have made a new group at the top called "Common" etc. for parameters like this.
There was a problem hiding this comment.
Yes, but there are two parameters: params.non_directional which is common between Bismark and BISCUIT, and params.min_coverage which is common between bwa-meth and BISCUIT.
Would you still do "Common" group for such a case?
There was a problem hiding this comment.
Yeah I think so - better to restructure the schema / docs rather than have similar but differently named parameters in my opinion 👍🏻
There was a problem hiding this comment.
Done 👍
Though there's a problem in the help message, I don't know how to solve it.
Copying here the "common option" group help message:
common options
--min_depth [integer] Specify a minimum read coverage for MethylDackel to report a methylation call in bwa-meth workflow, or a minimum
read coverage for information extraction from the VCF file to bed file in BISCUIT workflow.
--non_directional [boolean] Run alignment against all four possible strands.
There's a whitespace between "minimum" and "read" and I don't know what causes this problem
ewels
reviewed
Mar 30, 2021
|
Hi @ekushele, I just had a quick look over your PR here and fixed the merge conflicts that had arisen / did a tiny bit of whitespace tidying. So to recap, where are we this now?
Is that right? I'm trying to work out whether we should try to get this done before the DSL2 conversion of the pipeline (#199). In some ways the DSL2 conversion could actually make things easier - separate containers means less trouble with conflicting conda environment packages. But it will require a substantial rewrite of this code, notably to use centralised DSL2 module wrappers in nf-core/modules. This will need to be done for the DSL2 conversion anyway though, it's just a question of whether we try to get it merged and released prior to this. Given that the bioconda issue would be solved by DSL2, I'm inclined to do that first. What do you think? Would that be ok? Phil |
…brary and min_coverage
|
|
Replying to this: #186 (comment) Edit: |
|
closing this in favor of #295 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
nf-core/methylseq pull request
This is a continuation-improved PR of this PR: #147
PR checklist
nextflow run . -profile test,docker).nf-core lint .).docsis updatedCHANGELOG.mdis updatedREADME.mdis updatedLearn more about contributing: CONTRIBUTING.md