Fix/featurecounts mixed se pe - #3
Open
NicoDeVeaux wants to merge 16 commits into
Open
Conversation
NicoDeVeaux
force-pushed
the
fix/featurecounts-mixed-se-pe
branch
3 times, most recently
from
July 29, 2026 12:08
4910bdf to
90f1773
Compare
Merges the nf-core-bot template branch (nf-core#449, c26ac89) following the pattern used for 4.0.2 in nf-core#443: merge into a branch off dev and resolve there, rather than pushing to the bot's template branch. Only ro-crate-metadata.json conflicted. It is wholly regenerated on each build -- none of the six UUID @ids are shared between the two sides, and each is both a definition and a cross-reference -- so resolving it hunk-by-hunk risks references pointing at nodes taken from the other side. Taking the template side wholesale is also wrong: the bot generates its RO-Crate from the template README, so its description is boilerplate carrying "TODO nf-core" placeholders (7,179 chars against the real 13,620). Resolved by taking dev's file and regenerating with `nf-core pipelines rocrate`. Verified: description is the real pipeline documentation, 36 nodes, all 6 UUID references resolve, no dangling ids. .nf-core.yml and README.md auto-merged correctly, keeping the pipeline's lint configuration and real documentation while taking nf_core_version 4.0.3.
Restore subread to the registry version and drop the nf-core patch record. The pin was the stopgap for mixed single-end/paired-end consensus counting; the following commit handles mixed cohorts explicitly, so it is no longer needed. Reverts the pin introduced in nf-core#448.
…ing subread featureCounts (subread >= 2.1.0) applies paired-end mode (-p) per invocation and aborts when one invocation mixes single-end and paired-end BAMs. The consensus BAMs can span both library types, so split them by endedness, count each homogeneous batch with the correct pairing flag, then merge the per-batch matrices into a single consensus table for DESeq2 and MultiQC. The joined tuple arity differs between the two instantiations of this subworkflow: the merged-library caller carries a control-BAM column ([ meta, bam, control, peak ]) while the merged-replicate caller does not ([ meta, bams, peak ]). Index positionally to stay tolerant of both shapes, as the pre-split implementation did. The split replaced the single sorted BAM list from nf-core#448 with two per-endedness batches plus a merge step, each assembled from an unordered channel collect, so all three are sorted by filename. Without that the featureCounts column order, the merged matrix and its snapshot md5 stop being reproducible across runs and hosts — the guarantee nf-core#448 established. Gating is unchanged: the join with ch_peaks keeps only samples that contributed peaks, and combining with MACS3_CONSENSUS.out.saf still requires a consensus to exist (>= 2 samples).
Add unit coverage for the FEATURECOUNTS_MERGE script (per-batch matrices merged into one table, plus the SE/PE fixture pair) and pipeline-level tests for the all-single-end and all-paired-end cohorts, so both homogeneous paths are exercised alongside the mixed default. Each pipeline test snapshots versions.yml, as nf-core lint's nf_test_content rule requires. Snapshots regenerated on GitHub-hosted ubuntu-latest x86_64 runners so container digests and architecture match CI. This also corrects the committed FEATURECOUNTS_MERGE snapshot, which recorded an empty `sed` version because it had been generated on macOS where `sed --version` yields nothing.
The SE/PE split publishes *.SE.featureCounts.tsv and *.PE.featureCounts.tsv alongside the merged matrix; document them and why they exist. Also correct the merged matrix's extension, which the docs still gave as .txt.
NicoDeVeaux
force-pushed
the
fix/featurecounts-mixed-se-pe
branch
from
August 3, 2026 13:34
90f1773 to
38dc9e3
Compare
Also fix the nf-core#443 entry, which linked to nf-core/chipseq instead of nf-core/atacseq. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`collect` emits in task-completion order, and plot_macs3_qc.r / plot_homer_annotatepeaks.r derive sample ids from the order of the file list they are handed. In plot_homer_annotatepeaks.r the summary column order follows the `feature` factor levels, which accumulate by first appearance across the input files, so a reshuffle changes the md5 of macs3_annotatePeaks.*.summary.txt, its _mqc.tsv, and the MultiQC peak annotation plot table. Observed as an intermittent snapshot mismatch in tests/skip_trimming.nf.test on nf-core#453. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`toSortedList` emits an empty list when its source channel is empty, whereas `collect` emits nothing at all, so the previous version started PLOT_MACS3_QC with no files when no merged-replicate peaks exist and plot_macs3_qc.r died on `flag "i" requires an argument`. Seen in the `bowtie2 with stub` test. Sort the staged file list in the process script instead, which leaves the channel semantics untouched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…k-qc-plot-order Sort peak QC plot inputs deterministically
Resolves the ro-crate-metadata.json conflict by taking dev's copy: the two sides are identical apart from generated UUIDs and timestamps. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Keeps the new code strict-syntax clean, as nf-core#446 established. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Under subread 2.1.1 (which this PR restores by dropping the 2.0.1 pin from nf-core#448), bare '-p' only declares paired-end input and counts individual reads; counting fragments now requires '--countReadPairs'. Without it, consensus peak counts for paired-end libraries roughly double relative to the 2.0.1 behaviour, inflating DESeq2 size factors and all downstream numbers. Add '--countReadPairs' to the paired-end branch of the vendored subread/featurecounts module via an nf-core modules patch.
Under subread 2.1.1 (which this PR restores by dropping the 2.0.1 pin from nf-core#448), bare '-p' only declares paired-end input and counts individual reads; counting fragments now requires '--countReadPairs'. Without it, consensus peak counts for paired-end libraries roughly double relative to the 2.0.1 behaviour, inflating DESeq2 size factors and all downstream numbers. Add '--countReadPairs' to the paired-end branch of the vendored subread/featurecounts module via an nf-core modules patch.
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
3 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.
copy of nf-core#452, with the benefit of stacking to minimize changed lines