Documentation#106
Conversation
|
There was a problem hiding this comment.
Pull request overview
This PR primarily updates pipeline documentation and dependencies, removes debug code, and makes several configuration and module updates.
Key Changes
- Updated documentation in
docs/usage.mdanddocs/output.mdwith comprehensive parameter descriptions and output structure - Removed debug
.view()calls fromworkflows/lrsomatic.nf - Updated module versions (MultiQC 1.30→1.32, VEP, LongPhase, etc.)
- Added help message support via nf-schema plugin
- Updated Nextflow version requirement (24.10.5→25.04.0)
Reviewed changes
Copilot reviewed 49 out of 50 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| workflows/lrsomatic.nf | Removed debug .view() statements for cleaner code |
| docs/usage.md | Added comprehensive parameter documentation and usage examples |
| docs/output.md | Complete rewrite with detailed output structure documentation |
| modules/local/clairs/main.nf | Added indel calling flag and haplotagging skip option |
| subworkflows/nf-core/utils_nfschema_plugin/main.nf | Added help message functionality |
| subworkflows/local/tumor_normal_happhase.nf | Added size: 2 to groupTuple() for paired samples |
| modules.json | Updated module versions and git SHAs |
| nextflow.config | Updated minimum Nextflow version requirement |
| Multiple test files | Updated snapshots and test configurations |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
| // | ||
| def softwareVersionsToYAML(ch_versions) { | ||
| return ch_versions.unique().map { version -> processVersionsFromYAML(version) }.unique().mix(Channel.of(workflowVersionToYAML())) | ||
| return ch_versions.unique().map { version -> processVersionsFromYAML(version) }.unique().mix(channel.of(workflowVersionToYAML())) |
There was a problem hiding this comment.
Incorrect case for channel factory method: channel.of() should be Channel.of() (capital C). The lowercase version is deprecated and may cause issues.
| main: | ||
|
|
||
| ch_versions = Channel.empty() | ||
| ch_versions = channel.empty() |
There was a problem hiding this comment.
Inconsistent casing for channel factory method: should be Channel.empty() (capital C) instead of channel.empty() to match Nextflow conventions and other usages in the codebase.
| // | ||
|
|
||
| Channel | ||
| channel |
There was a problem hiding this comment.
Inconsistent casing for channel factory method: should be Channel.fromList() (capital C) instead of channel.fromList() to match Nextflow conventions and other usages in the codebase.
| return[new_meta, [[type: meta.type], hapbam], [[type: meta.type], hapbai]] | ||
| } | ||
| .groupTuple() | ||
| .groupTuple(size: 2) |
There was a problem hiding this comment.
The size: 2 parameter in groupTuple() assumes exactly 2 samples (tumor and normal). This could cause the pipeline to hang or fail if a sample doesn't have both tumor and normal data. Consider adding error handling or documenting this requirement clearly.
| --normal_bam_fn $normal_bam \\ | ||
| --ref_fn $reference \\ | ||
| --threads $task.cpus \\ | ||
| --enable_indel_calling \\ |
There was a problem hiding this comment.
The new flag --haplotagged_tumor_bam_provided_so_skip_intermediate_phasing_and_haplotagging is added without documentation. Consider adding a comment explaining why this flag is needed and what assumptions it makes about the input data.
| --enable_indel_calling \\ | |
| --enable_indel_calling \\ | |
| # The following flag indicates that the input tumor BAM file is already haplotagged. | |
| # This allows the pipeline to skip intermediate phasing and haplotagging steps. | |
| # Assumes that the input BAM is properly haplotagged and suitable for downstream analysis. |
Co-authored-by: ljwharbers <40829819+ljwharbers@users.noreply.github.com>
Add bibliography of tools used in pipeline
ljwharbers
left a comment
There was a problem hiding this comment.
After adding wakhan and multiqc in output description, ready to go.
PR checklist
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).