Conversation
This PR is against the
|
|
There was a problem hiding this comment.
Pull request overview
This is a hotfix PR that adds ASCAT PDF plotting as a configurable option and fixes channel structure issues for multiple custom model downloads. The PR also includes version bump to v1.1.0dev and updates project metadata with the official Zenodo DOI.
- Adds configurable PDF plot generation for ASCAT copy number analysis
- Fixes basecall_model handling in tumor/normal sample processing to properly use clair3_model when specified
- Updates version from v1.0.0 to v1.1.0dev with corresponding metadata changes
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| workflows/lrsomatic.nf | Adds debug view() statements (should be removed) |
| subworkflows/local/tumor_normal_happhase.nf | Fixes basecall_model assignment to use clair3_model when available; adds multiple debug view() statements (should be removed) |
| modules/nf-core/ascat/main.nf | Adds pdf_plots parameter to ASCAT runAscat calls, enabling configurable PDF output |
| nextflow.config | Adds ascat_pdf_plots parameter (default "False") and updates version to v1.1.0dev |
| conf/modules.config | Passes pdf_plots parameter to ASCAT process configuration |
| tests/default.nf.test.snap | Updates version reference and test timestamp |
| ro-crate-metadata.json | Updates version, URLs, status, and UUIDs for v1.1.0dev |
| README.md | Updates Zenodo DOI from placeholder to actual DOI (10.5281/zenodo.17751829) |
| CHANGELOG.md | Adds v1.1.0 section and marks v1.0.0 release date |
| .nf-core.yml | Updates template version to v1.1.0dev |
| assets/multiqc_config.yml | Updates report link from v1.0.0 release to dev branch |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return[new_meta, bam, bai] | ||
| } | ||
| .set{ tumor_bams } | ||
| tumor_bams.view() |
There was a problem hiding this comment.
Debug view() statement should be removed before merging. This appears to be temporary debugging code left in the subworkflow.
| tumor_bams.view() |
| somatic_vep = Channel.empty() | ||
| germline_vep = Channel.empty() | ||
|
|
||
| mixed_bams.view() |
There was a problem hiding this comment.
Debug view() statement should be removed before merging. This appears to be temporary debugging code left in the subworkflow.
| } | ||
| .set{downloaded_model_files} | ||
|
|
||
| downloaded_model_files.view() |
There was a problem hiding this comment.
Debug view() statement should be removed before merging. This appears to be temporary debugging code left in the subworkflow.
| downloaded_model_files.view() |
| return [ basecall_model, new_meta, bam, bai ] | ||
| } | ||
| .set { normal_bams_model } | ||
| normal_bams_model.view() |
There was a problem hiding this comment.
Debug view() statement should be removed before merging. This appears to be temporary debugging code left in the subworkflow.
| return [meta, bam, bai, model, platform] | ||
| } | ||
| .set{ normal_bams } | ||
| normal_bams.view() |
There was a problem hiding this comment.
Debug view() statement should be removed before merging. This appears to be temporary debugging code left in the subworkflow.
| tumor_bams.view() | ||
| LONGPHASE_PHASE.out.vcf.view() |
There was a problem hiding this comment.
Debug view() statements should be removed before merging. These appear to be temporary debugging code left in the subworkflow.
| tumor_bams.view() | |
| LONGPHASE_PHASE.out.vcf.view() |
| wakhan_input.view() | ||
| ch_fasta.view() |
There was a problem hiding this comment.
Debug view() statements should be removed before merging. These appear to be temporary debugging code left in the workflow and will clutter the execution logs with channel contents.
| wakhan_input.view() | |
| ch_fasta.view() |
| ascat.output <- ascat.runAscat(ascat.bc, gamma=1, rho_manual=$purity, psi_manual=$ploidy, pdfPlot = "$pdf_plots") | ||
| } else if(!is.null($purity) && is.null($ploidy)){ | ||
| ascat.output <- ascat.runAscat(ascat.bc, gamma=1, rho_manual=$purity) | ||
| ascat.output <- ascat.runAscat(ascat.bc, gamma=1, rho_manual=$purity, pdfPlot = "$pdf_plots") | ||
| } else if(!is.null($ploidy) && is.null($purity)){ | ||
| ascat.output <- ascat.runAscat(ascat.bc, gamma=1, psi_manual=$ploidy) | ||
| ascat.output <- ascat.runAscat(ascat.bc, gamma=1, psi_manual=$ploidy, pdfPlot = "$pdf_plots") | ||
| } else { | ||
| ascat.output <- ascat.runAscat(ascat.bc, gamma=1) | ||
| ascat.output <- ascat.runAscat(ascat.bc, gamma=1, pdfPlot = "$pdf_plots") |
There was a problem hiding this comment.
The value of pdf_plots coming from args.pdf_plots is interpolated directly into the R script inside a quoted string (pdfPlot = "$pdf_plots"), so a malicious CLI parameter containing double quotes can break out of the string literal and inject arbitrary R code (for example by calling system()) in the ASCAT container. This means anyone who can control pipeline parameters can achieve code execution in the context of this process. Please constrain args.pdf_plots to a small, fixed set of safe values (e.g. a boolean) or ensure it is properly escaped/sanitized before interpolation, and apply the same hardening pattern to any other task.ext.args that are inserted into R code.
Adds ASCAT pdf printing as option and slight channel structure fix for multiple custom model downloads.
nf-core pipelines lint).nextflow run . -profile test,docker --outdir <OUTDIR>).nextflow run . -profile debug,test,docker --outdir <OUTDIR>).docs/usage.mdis updated.docs/output.mdis updated.CHANGELOG.mdis updated.README.mdis updated (including new tool citations and authors/contributors).