Add coverm/contig to nf-core/modules by vinisalazar · Pull Request #997 · nf-core/mag

vinisalazar · 2026-03-12T02:13:15Z

Add CoverM to calculate read alignment metrics for contig/bin alignments (#587)

Run nf-core modules install coverm/contig
Create new module directory and modify modules.json

PR checklist

vinisalazar · 2026-03-12T02:13:57Z

Converting this to draft until I add the module logic to the subworkflows, will ping when done.

vinisalazar · 2026-03-12T04:08:15Z

Update to changes:

in bin/get_mag_depths.py, add option to open gzip and non-gzip files
contig depth calculation by default is now done with COVERM_CONTIG (METABAT2 is still an option)
on mag_depths_convert/main.nf, define function to remove gz extension
Add new params to nextflow.config with coverm options
Update nextflow_schema.json with relevant new params
On subworkflows/local/binning/main.nf:
- Add checks to make sure CoverM mapper is bowtie2 (otherwise doesn't generate BAM files required for binning)
- Enable both METABAT2 and COVERM as contig depth calculators
  - Same is done on subworkflows/local/binning_preparation for SR and LR
- Rename output channel from metabat2depths to contig_depths

vinisalazar · 2026-03-12T05:09:08Z

This one is good to go but will require merging nf-core/modules#10588 for the lint checks to pass.

vinisalazar · 2026-03-12T05:10:29Z

PS all code generated by @claude was reviewed

vinisalazar · 2026-03-12T11:51:30Z

Making this into draft again until the upstream update to the coverm module is merged.

Add CoverM to calculate read alignment metrics for contig/bin alignments (nf-core#587) - Run `nf-core modules install coverm/contig` - Create new module directory and modify modules.json

Introduces --coverm_mapper (default: 'bowtie2') to control which aligner is used when estimating contig depths. CoverM-native mappers skip bowtie2 alignment and let CoverM handle reads→depths directly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

- COVERM_CONTIG_SHORTREAD/LONGREAD replace both METABAT2_JGISUMMARIZEBAMCONTIGDEPTHS variants - bowtie2 path: BAMs → CoverM (bam_input=true) - coverm-native path: reads+assembly → CoverM (bam_input=false) - Add runtime error when BAM-requiring binners are used with coverm-native - Rename emit metabat2depths → contig_depths Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Add COVERM_CONTIG_SHORTREAD and COVERM_CONTIG_LONGREAD blocks with --methods metabat to output MetaBAT2-compatible depth format. Shortread also passes --mapper for coverm-native aligner selection. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

CoverM outputs uncompressed depth files; update get_mag_depths.py and CONVERT_DEPTHS to accept both .gz and plain-text inputs. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Replace METABAT2_JGISUMMARIZEBAMCONTIGDEPTHS references with COVERM_CONTIG and update depth file extension from .txt.gz to .txt. Snapshots will need regeneration after the first test run to capture updated version strings. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

- Add --depth_calculator (coverm|metabat2, default: coverm) to switch between COVERM_CONTIG and METABAT2_JGISUMMARIZEBAMCONTIGDEPTHS for contig depth estimation - Add --coverm_methods with full CoverM methods enum (default: metabat) to configure the coverage calculation method passed to CoverM - Restore METABAT2_JGISUMMARIZEBAMCONTIGDEPTHS_SHORTREAD/LONGREAD config blocks in modules.config Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

- Update coverm/contig to use eval/topic-based versioning and handle methods as List or string; remove versions.yml generation - Remove ch_versions.mix for COVERM_CONTIG (topic versioning is automatic) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

- Sync coverm/contig main.nf and meta.yml to reference (eval/topic versioning, List-aware methods string handling) - Remove ch_versions.mix for COVERM_CONTIG calls (topic versioning is automatic; no versions.yml emitted) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

- Add coverm to dependencies - Add nf-core#997 to 'Added' section

- Reflect addition of COVERM_CONTIG as contig depth calculator

- Use version topics

vinisalazar · 2026-03-16T23:55:20Z

This is now ready for review, but changes are extensive so some advice on testing would be welcome.

Thanks

jfy133 · 2026-03-17T10:25:22Z

Thanks @vinisalazar !

I need to focus on taxprofiler for the next few weeks... @dialvarezs do you think you can go through this?

prototaxites

Hi @vinisalazar, this looks really good! I have a few thoughts on the implementation that I think it's worth having a discussion about?

prototaxites · 2026-03-17T10:31:17Z

subworkflows/local/binning/main.nf

+    // Validate: BAM-requiring binners cannot be used with CoverM native mappers
+    // because no BAM files are produced in that path


CoverM will output BAM files if --bam-file-cache-directory is set! We should force that option so we can get them.

Sounds good to me

prototaxites · 2026-03-17T10:31:43Z

subworkflows/local/binning/main.nf

-            [meta, bams, bais]
+    // Validate: BAM-requiring binners cannot be used with CoverM native mappers
+    // because no BAM files are produced in that path
+    if (params.coverm_mapper != 'bowtie2') {


Would suggest we call this param coverage_mapper?

prototaxites · 2026-03-17T10:32:40Z

subworkflows/local/binning/main.nf

-    METABAT2_JGISUMMARIZEBAMCONTIGDEPTHS_LONGREAD(ch_summarizedepth_input.longread)
-    ch_versions = ch_versions.mix(METABAT2_JGISUMMARIZEBAMCONTIGDEPTHS_LONGREAD.out.versions)
+    // Long reads always come as BAMs (from minimap2)
+    if (params.depth_calculator == 'coverm') {


IMO, though others may disagree, we should just drop JGISUMMARIZEBAMCONTIGDEPTHS entirely and replace it with CoverM. The outputs in "metabat" mode should be value-for-value identical, and supporting both requires a lot of complicated control flow.

I agree with this. If CoverM produces a equivalent output, then keeping it it's just unnecessary complexity.

I would be happy to implement this. Would you need me to rebase this branch and edit the commits I've already made, or is it okay to just add commits on top?

Commits on top, I'm personally not fussy about super clean git history (unless @dialvarezs @prototaxites have a stronger opinion)

No, fine with me too. Can always do a squash commit if things are really bad!

Note I'm the person who is often reluctant to deprecate existing/accepted modules, and often rather prefer keep adding new altenrative options unless there is a very very clear case of something being superior.

But like @dialvarezs says - if the output is the exact same then I'm fine with the older module being deprecated.

prototaxites · 2026-03-17T10:34:37Z

subworkflows/local/binning_preparation_shortread/main.nf

+ * Binning preparation for short reads
 */

 include { BOWTIE2_ASSEMBLY_BUILD } from '../../../modules/local/bowtie2_assembly_build/main'


Question, would it make sense to move CoverM into this workflow (and drop the long reads one) so that we have a unified place where all coverage estimation happens?

That makes sense to me too. l'll see how this could be done when working on the "drop the JGIBAMSUMMARIZE..." part.

vinisalazar · 2026-03-18T02:50:09Z

Thanks @prototaxites @dialvarezs @jfy133 for your comments. I have limited time to work on this, but I will try to put in some effort in the next 1-2 weeks.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

CoverM in --methods metabat mode produces value-for-value identical output, so JGISUMMARIZEBAMCONTIGDEPTHS and the depth_calculator param are now removed. CoverM is always used for contig depth estimation. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…identity shortread_percentidentity and longread_percentidentity previously set --percentIdentity on JGISUMMARIZEBAMCONTIGDEPTHS; now they pass --min-read-percent-identity to COVERM_CONTIG_SHORTREAD and COVERM_CONTIG_LONGREAD respectively. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

When using a CoverM-native mapper (non-bowtie2), pass --bam-file-cache-directory so that CoverM caches the BAM files it generates internally during alignment. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Remove references to the dropped depth_calculator param and jgi_summarize_bam_contig_depths. Rename coverm_mapper → coverage_mapper throughout. Update percentidentity help text to reference CoverM's --min-read-percent-identity flag. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

vinisalazar · 2026-03-24T03:13:26Z

@prototaxites @dialvarezs implemented the changes requested here, starting from e5b6fa2. However, I'm having a hard time interpreting the tests. The ones that passed in the CI still show errors when inspecting the logs, and others just failed.

I ran the nf-test command and manually inspected the output files and they look right to me, here are the CoverM output file and the MaxBin2 summary file:

I will try to debug to see how to get these tests to pass, but open to suggestions if you have any.

dialvarezs · 2026-03-24T19:26:06Z

Hey @vinisalazar, I'm going to give a look at this tomorrow.

vinisalazar requested review from d4straub, dialvarezs, jfy133, muabnezor and prototaxites as code owners March 12, 2026 02:13

vinisalazar marked this pull request as draft March 12, 2026 02:13

vinisalazar force-pushed the dev branch from 36d770a to 83ba4e5 Compare March 12, 2026 05:05

vinisalazar marked this pull request as ready for review March 12, 2026 05:08

vinisalazar marked this pull request as draft March 12, 2026 11:50

vinisalazar force-pushed the dev branch 2 times, most recently from c0d42d6 to 55f610f Compare March 12, 2026 12:56

vinisalazar and others added 14 commits March 17, 2026 10:50

Add coverm/contig to nf-core/modules

19162fa

Add CoverM to calculate read alignment metrics for contig/bin alignments (nf-core#587) - Run `nf-core modules install coverm/contig` - Create new module directory and modify modules.json

Handle plain-text (non-gzipped) depth files from CoverM

3b9c055

CoverM outputs uncompressed depth files; update get_mag_depths.py and CONVERT_DEPTHS to accept both .gz and plain-text inputs. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

CHANGELOG: fix conflict

04de852

- Add coverm to dependencies - Add nf-core#997 to 'Added' section

docs: update usage.md and output.md

e4fa214

- Reflect addition of COVERM_CONTIG as contig depth calculator

CITATIONS.md: add coverm

ec84e7c

README.md: add coverm

c7e509f

coverm/contig: update test snapshot

eb5ab55

- Use version topics

coverm/contig: update module

c8f2649

vinisalazar force-pushed the dev branch from 55f610f to c8f2649 Compare March 16, 2026 23:53

vinisalazar marked this pull request as ready for review March 16, 2026 23:54

Update coverm version in test snapshots

d66a3bb

prototaxites reviewed Mar 17, 2026

View reviewed changes

vinisalazar and others added 6 commits March 24, 2026 11:51

rename: coverm_mapper param → coverage_mapper

e5b6fa2

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

feat: add --bam-file-cache-directory to CoverM native mapper config

e8d772b

When using a CoverM-native mapper (non-bowtie2), pass --bam-file-cache-directory so that CoverM caches the BAM files it generates internally during alignment. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

docs: add CoverM to pipeline summary in ro-crate metadata

ef5ea46

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

vinisalazar requested a review from prototaxites March 24, 2026 03:09

		// Validate: BAM-requiring binners cannot be used with CoverM native mappers
		// because no BAM files are produced in that path

Conversation

vinisalazar commented Mar 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR checklist

Uh oh!

vinisalazar commented Mar 12, 2026

Uh oh!

vinisalazar commented Mar 12, 2026

Uh oh!

vinisalazar commented Mar 12, 2026

Uh oh!

vinisalazar commented Mar 12, 2026

Uh oh!

vinisalazar commented Mar 12, 2026

Uh oh!

vinisalazar commented Mar 16, 2026

Uh oh!

jfy133 commented Mar 17, 2026

Uh oh!

prototaxites left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vinisalazar commented Mar 18, 2026

Uh oh!

vinisalazar commented Mar 24, 2026

Uh oh!

dialvarezs commented Mar 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

vinisalazar commented Mar 12, 2026 •

edited

Loading