Port subworkflow rmats to nf-core structure - #291
Merged
Merged
Conversation
Add meta.yml and nf-tests for the local RMATS subworkflow. nf-core/modules has rmats/prep, which the subworkflow already uses, but no rmats/post module and no rMATS subworkflow, so CREATE_BAMLIST and RMATS_POST stay local. The tests cover two contrasts sharing their samples with the unpaired model, one contrast with the paired model, a single condition, a paired contrast with unequal sample counts and a stub run. Also: - Take the genome BAM files as [ meta, bam ] and read the condition from meta.condition, instead of [ condition, meta, bam ] tuples built by the caller. - Build the samples of every rMATS run from the samplesheet order, the same way for the paired and unpaired models, which replaces the multiMap and the join on sample positions. - Stop with an error when a paired contrast has conditions of different sizes. The join dropped the samples with no counterpart, so the existing size check could never fire. A contrast naming a condition with no sample also stops with an error instead of being skipped. - Leave the control key out of the meta map of a single condition run. An empty string made nf-test hash the whole working directory as a path. - Emit the bam lists, add the MODULE section headers and format with nextflow lint -format. Generated by Claude Opus 5.5 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
The single condition test gave RMATS_POST a meta map with control: ''. nf-test reads a string naming an existing file as a path, and '' is the working directory, so every snapshot comparison and save hashed the whole repository: the test file took 406s instead of 66s and printed EOFException traces on partly written .gz files. The meta map now has no control key, as the RMATS subworkflow builds it for a single condition. Generated by Claude Opus 5.5 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
❌ nf-test failed with latest Nextflow versionNote Tests with Nextflow's latest version failed but it will not cause a CI workflow failure.
See the full run for details. |
erikrikarddaniel
approved these changes
Sep 24, 2026
erikrikarddaniel
left a comment
erikrikarddaniel
left a comment
Member
There was a problem hiding this comment.
Reviewed with Claude Code.
This looks good to me. The contrast samples are now built one way for both models, and the size check runs on the samplesheet. That means the silent drop of unmatched samples is gone. I checked two things that could break the positional pairing, and both are safe. ch_genome_bam is never filtered before rMATS. The genome_bam schema enforces unique sample ids. The paired and unpaired snapshots also differ, so --paired-stats really takes effect in the tests.
Two small inline comments, neither blocking.
One optional thought for later: the new "condition has no sample" check only runs with --rmats. The same contrastsheet then passes silently with the other tools. It could move next to validateInputContrastsheet in PIPELINE_INITIALISATION, so that every tool gets it. That is out of scope for this PR.
| .map { row, by_condition -> | ||
| [row.treatment, row.control].each { condition -> | ||
| if (!by_condition.containsKey(condition)) { | ||
| error("rMATS contrast '${row.contrast}': condition '${condition}' has no sample in the samplesheet") |
Member
There was a problem hiding this comment.
This new error has no test. A failure test like paired - unequal sample counts would cover it: a contrast naming a condition that has no sample, and an assert on workflow.stdout.
Comment on lines
+71
to
+72
| Channel containing the comma separated list of the control BAM files of each contrast, | ||
| empty in single condition mode |
Member
There was a problem hiding this comment.
bam_list2 is an optional output, so a single condition run emits no entry rather than an empty one:
Suggested change
| Channel containing the comma separated list of the control BAM files of each contrast, | |
| empty in single condition mode | |
| Channel containing the comma separated list of the control BAM files of each contrast, | |
| with no entry in single condition mode |
Author
|
Thanks @erikrikarddaniel ! You are right, in the next PR I'll make the contrasts consistent. |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Description
Moves the local
RMATSsubworkflow to the nf-core subworkflow template, the same wayPREPARE_GENOMEwas ported in #290. nf-core/modules hasrmats/prep, which the subworkflow already uses, but normats/postmodule and no rMATS subworkflow, soCREATE_BAMLISTandRMATS_POSTstay local.Changes
meta.yml. Authors are taken from the git history of the subworkflow (@asmaali98, @bensouthgate, @jma1991), plus @piplus2 as author and maintainer.[ meta, bam ]and reads the condition frommeta.condition. The caller no longer builds[ condition, meta, bam ]tuples.multiMapand the join on sample positions, and the bam lists now always follow the samplesheet order.nextflow lint -format.Fixes
--rmats_paired_stats, a contrast whose conditions have different numbers of samples dropped the samples with no counterpart without any message: the join discarded them before the existing size check ran, so the check could never fire. It now stops with an error naming the samples of both conditions.control: ''. It now leaves the key out. nf-test reads a string naming an existing file as a path, and''is the working directory, so every snapshot save hashed the whole repository.Testing
nf-core pipelines lint(tools 4.1.0) 0 failures,prekclean.nf-core pipelines lintwarns thatcreate/bamlistandrmats/postare missing from the subworkflowmeta.yml. They are listed under their real names,create_bamlistandrmats_post; the linter splits local module names on underscores, the same false positive as inPREPARE_GENOMEandLEAFCUTTER.single conditiontest of the localRMATS_POSTmodule had the samecontrol:''meta map. It now leaves the key out too, which brings the module test file from 406s down to 66s; only the meta map in its snapshot changes.Generated by Claude Opus 5.5
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).🤖 Generated with Claude Code