Skip to content

New module: modkit/extractcalls - #11286

Merged
SPPearce merged 5 commits into
nf-core:masterfrom
sahuno:add-modkit-extract-calls
Sep 11, 2026
Merged

New module: modkit/extractcalls#11286
SPPearce merged 5 commits into
nf-core:masterfrom
sahuno:add-modkit-extract-calls

Conversation

@sahuno

@sahuno sahuno commented Apr 24, 2026

Copy link
Copy Markdown
Contributor

PR checklist

  • This comment contains a description of changes (with reason).
  • Stub + real tests added.
  • New tool follows module conventions.
  • Versions broadcast via `topic: versions`.
  • Naming/parameter/I-O conventions followed.
  • Resource label set (`process_high`).
  • BioConda + BioContainers used.
  • `nf-core modules lint modkit/extract/calls` — 49/0/0.
  • `nf-test test --profile conda` — 2/2 passed.

Summary

Adds a new nf-core module wrapping `modkit extract calls`, which produces a per-read per-position table of base-modification calls (pass / fail / filtered, with the called base) using the same thresholding algorithm as `modkit pileup`.

Complementary to `modkit/extract/full`: `extract calls` emits the thresholded categorical decision per site per read, while `extract full` emits the underlying probabilities.

The module auto-detects `--bgzf` in `ext.args` and adjusts the output filename suffix accordingly.

Why

`modkit extract calls` is the go-to tool for per-read allele-specific methylation, methylation-aware phasing validation, and read-level QC where you want the same thresholded labels as the pileup output but at read level rather than site level.

Test data

Uses the existing `test.sorted.phased.bam` from nf-core/test-datasets (modules branch). No new test data required.

🤖 Generated with Claude Code

@sahuno
sahuno force-pushed the add-modkit-extract-calls branch from 5035e89 to fafc527 Compare April 24, 2026 02:44
@SPPearce
SPPearce added this pull request to the merge queue Jun 6, 2026
@SPPearce
SPPearce removed this pull request from the merge queue due to a manual request Jun 6, 2026

@SPPearce SPPearce left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is named incorrectly, it is too nested in the subfolder.
So modkit/foo, but not modkit/foo/bar

@SPPearce

SPPearce commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

So modkit/extractcalls here

@atrigila atrigila added the awaiting-changes will be closed after 30 days label Jul 2, 2026
@github-actions

Copy link
Copy Markdown

This PR has been tagged as awaiting-changes or awaiting-feedback by an @nf-core/modules contributor. Remove stale label or add a comment if it is still useful.

@github-actions github-actions Bot added the stale Stale label Aug 16, 2026
@sahuno sahuno changed the title New module: modkit/extract/calls New module: modkit/extractcalls Sep 6, 2026
@sahuno sahuno mentioned this pull request Sep 6, 2026
9 tasks
@sahuno

sahuno commented Sep 6, 2026

Copy link
Copy Markdown
Contributor Author

@SPPearce thanks — renamed to modkit/extractcalls.

modkit/extract/calls was three levels deep, which fails the
main_nf_module_granularity lint check (<tool> or <tool>/<subtool> only).
Renamed the directory, the process name (MODKIT_EXTRACTCALLS), the meta.yml
name: field and the nf-test tags to match.

Sibling PR #11285 (modkit/extractfull) has had the same rename applied.

@sahuno sahuno removed the stale Stale label Sep 6, 2026

@SPPearce SPPearce left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally fine now, please just tidy up the snapshot generation.

Comment thread modules/nf-core/modkit/extractcalls/tests/main.nf.test
Comment thread modules/nf-core/modkit/extractcalls/tests/main.nf.test
Comment thread modules/nf-core/modkit/extractcalls/meta.yml Outdated
Comment thread modules/nf-core/modkit/extractcalls/tests/nextflow.config Outdated
sahuno and others added 3 commits September 7, 2026 05:55
Add new nf-core module wrapping `modkit extract calls`, which emits a
per-read per-position table of base modification calls using the same
pass/fail thresholding as `modkit pileup`. Complementary to
`modkit/extract/full` (raw probabilities): this module emits the
thresholded categorical decisions. Useful for per-read downstream
analysis such as allele-specific methylation and methylation-aware
phasing.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
nf-core module names must be <tool> or <tool>/<subtool> (two levels max);
`modkit/extract/calls` failed the `main_nf_module_granularity` lint check.

Renamed directory, process name (MODKIT_EXTRACTCALLS), meta.yml name and
nf-test tags accordingly, as requested by @SPPearce.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01E7RSvpouxbMBdimYSwJ5ZP
… empty config

- Snapshot via sanitizeOutput(process.out) with `assert process.success`
  outside assertAll, so a tool failure fails fast instead of reporting
  snapshot mismatches against empty output.
- Drop the redundant exists/size asserts in the non-stub test; the snapshot
  already covers the tsv, and an empty file is caught by the md5 lint check.
- meta.yml: the index may be a .crai — modkit extract calls reads CRAM
  (verified against modkit 0.6.1 on a CRAM built from the test modBAM).
- Remove tests/nextflow.config (it only set an empty ext.args) and its
  config directive. Non-stub tsv md5 is unchanged: 0ce2e5a6...

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SHRC2SxQjmF3X1z82LopQb
@sahuno
sahuno force-pushed the add-modkit-extract-calls branch from 743f31b to 559d50c Compare September 7, 2026 10:03
@sahuno

sahuno commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

@SPPearce all four addressed, thanks.

First a note that explains why the diff is bigger than the four comments: the
branch was ~980 commits behind master, so sanitizeOutput was not available on
it at all — it comes from nft-utils 1.x and the branch's nf-test.config
still pinned nft-utils@0.0.9. Rebased onto current master before making the
changes.

  • Snapshot generation. Both tests now use your form: assert process.success
    outside assertAll, then a single snapshot(sanitizeOutput(process.out)).match().
    Snapshot regenerated — it drops the duplicated numbered keys and is about half
    the size.
  • Redundant asserts. Removed from the non-stub test.
  • .crai. You were right, and I checked rather than assumed: modkit extract calls reads CRAM. I built one from test.sorted.phased.bam against
    genome.fasta, indexed it, and got the same 27,813 rows and exit 0 as the BAM
    input. The index input is now pattern: "*.{bai,csi,crai}", and the bam
    description mentions that a CRAM input also needs fasta supplied.
  • tests/nextflow.config. Deleted, along with the config "./nextflow.config"
    directive that loaded it — it only set ext.args = ''. The non-stub tsv md5 is
    unchanged (0ce2e5a6…), which confirms it was a no-op rather than something the
    test quietly depended on.

Both nf-tests pass locally under the singularity profile.

One thing to flag: lint now warns bioconda_latest: ont-modkit 0.6.1 -> 0.6.4.
I have left it at 0.6.1 to stay consistent with the merged modkit/pileup and my
three sibling modkit PRs (#11283, #11284, #11285). Happy to bump all of them
together in a follow-up if you would rather they move to 0.6.4.

@SPPearce

SPPearce commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

At some point a bump to the newer version would be good, but they are better to be all on the same version as you say. Happy for a bump to occur within one of these new module PRs.

@SPPearce

SPPearce commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Ok, can merge this.
If you want, you can incorporate the version bump here.
Please update the other two modules with the same sanitizeOutput changes too (if you haven't already done this)

Clears the `bioconda_latest` lint warning. Container tag
`0.6.4--h7f49ad2_0` verified on both the Galaxy depot and quay.io.

Snapshot regenerated: only the reported version string changes. The
non-stub tsv md5 is identical to 0.6.1 (0ce2e5a6bf0889aaf8cbd682e2b17acb),
so `modkit extract calls` output is byte-stable across the bump.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JAsT7dzCcLSicJxFRWJozr
sahuno added a commit to sahuno/modules that referenced this pull request Sep 9, 2026
…o 0.6.4

Ports the review changes from nf-core#11286 to this module.

- Snapshot via `sanitizeOutput(process.out)` with `assert process.success`
  outside `assertAll`, so a tool failure fails fast rather than reporting a
  snapshot mismatch against empty output. Requires nft-utils 1.x, which is
  why master is merged in first.
- Drop the redundant exists/size asserts on the non-stub test; the snapshot
  already covers the tsv and an empty file is caught by the md5 lint check.
- Remove tests/nextflow.config (it only set an empty ext.args) and the
  `config` directive that loaded it.
- Bump ont-modkit 0.6.1 -> 0.6.4 (clears the `bioconda_latest` lint warning);
  container tag `0.6.4--h7f49ad2_0` verified on the Galaxy depot and quay.io.

Non-stub tsv md5 is unchanged at 8300986c98d1b67973439ced4e7f233c across both
the config removal and the version bump.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JAsT7dzCcLSicJxFRWJozr
sahuno added a commit to sahuno/modules that referenced this pull request Sep 9, 2026
….6.4

Ports the review changes from nf-core#11286 to this module.

- Snapshot via `sanitizeOutput(process.out)` with `assert process.success`
  outside `assertAll`, so a tool failure fails fast rather than reporting a
  snapshot mismatch against empty output. Requires nft-utils 1.x, which is
  why master is merged in first.
- tests/nextflow.config is kept here: unlike extractcalls/extractfull it sets
  a real `ext.args = '-c C'`, not an empty string.
- The raw_accuracy / filtered_accuracy / filter_threshold asserts are kept —
  they check report content the snapshot's md5 does not describe.
- Bump ont-modkit 0.6.1 -> 0.6.4 (clears the `bioconda_latest` lint warning);
  container tag `0.6.4--h7f49ad2_0` verified on the Galaxy depot and quay.io.

Report md5 is unchanged at 8503eee0403386ba18c24b0d584c7ee6 across the bump.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JAsT7dzCcLSicJxFRWJozr
sahuno added a commit to sahuno/modules that referenced this pull request Sep 9, 2026
Ports the review changes from nf-core#11286 to this module.

- `assert process.success` moved out of `assertAll` and to the top of each
  `then` block — ahead of the `path(...).readLines()` calls, which would
  otherwise throw on a failed run before any assertion reported.
- The two stub tests snapshot `sanitizeOutput(process.out)`, which drops the
  duplicated numbered output keys. Requires nft-utils 1.x, which is why
  master is merged in first.
- The two non-stub tests still snapshot only the versions topic: entropy's
  floating-point columns are not byte-deterministic across CPU counts, so
  those outputs stay asserted on structure rather than md5.
- tests/nextflow.config is kept: it sets a real `ext.args = '--cpg --header'`.
- Bump ont-modkit 0.6.1 -> 0.6.4 (clears the `bioconda_latest` lint warning);
  container tag `0.6.4--h7f49ad2_0` verified on the Galaxy depot and quay.io.

The header and column-count assertions pass unchanged at 0.6.4, so the
entropy BED/bedgraph layout is stable across the bump.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JAsT7dzCcLSicJxFRWJozr
sahuno added a commit to sahuno/modules that referenced this pull request Sep 9, 2026
….6.4

Ports the review changes from nf-core#11286 to this module, keeping all four open
modkit module PRs on one modkit version.

- Snapshot via `sanitizeOutput(process.out)` with `assert process.success`
  outside `assertAll`, so a tool failure fails fast rather than reporting a
  snapshot mismatch against empty output.
- tests/nextflow.config is kept: it carries a real `ext.args2 = '-p bed'` for
  HTSLIB_BGZIPTABIX, not an empty ext.args.
- Bump ont-modkit 0.6.1 -> 0.6.4 (clears the `bioconda_latest` lint warning);
  container tag `0.6.4--h7f49ad2_0` verified on the Galaxy depot and quay.io.

All three output md5s are unchanged across the bump
(bc5b3b2e…, fdabfb3e…, and the empty-file stub hash).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JAsT7dzCcLSicJxFRWJozr
sahuno added a commit to sahuno/modules that referenced this pull request Sep 9, 2026
….6.4

Ports the review changes from nf-core#11286 to this module, keeping all four open
modkit module PRs on one modkit version.

- Snapshot via `sanitizeOutput(process.out)` with `assert process.success`
  outside `assertAll`, so a tool failure fails fast rather than reporting a
  snapshot mismatch against empty output.
- tests/nextflow.config is kept: it carries a real `ext.args2 = '-p bed'` for
  HTSLIB_BGZIPTABIX, not an empty ext.args.
- Bump ont-modkit 0.6.1 -> 0.6.4 (clears the `bioconda_latest` lint warning);
  container tag `0.6.4--h7f49ad2_0` verified on the Galaxy depot and quay.io.

All three output md5s are unchanged across the bump
(bc5b3b2e…, fdabfb3e…, and the empty-file stub hash).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JAsT7dzCcLSicJxFRWJozr
@sahuno

sahuno commented Sep 11, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @SPPearce. Both done:

  • Version bump is in this PRff5d631 bumps ont-modkit 0.6.1 → 0.6.4 in environment.yml, and the snapshots were regenerated against it.
  • The sibling modules already carry the same sanitizeOutput changeNew module: modkit/extractfull #11285 (modkit/extractfull), New module: modkit/validate #11284 (modkit/validate), New module: modkit/entropy #11283 (modkit/entropy) and Add module: modkit/localize #11120 (modkit/localize) were each rebased onto master, switched to assert process.success + snapshot(sanitizeOutput(process.out)), had their snapshots regenerated and their containers bumped to 0.6.4. (modkit/entropy keeps its structural assertions on the non-stub tests — its float columns are not byte-deterministic across CPU counts — but the stubs use sanitizeOutput.)

The two review threads above are addressed and resolved. This one is green and mergeable whenever you are ready.

@sahuno sahuno mentioned this pull request Sep 11, 2026
3 tasks
@SPPearce
SPPearce added this pull request to the merge queue Sep 11, 2026
Merged via the queue into nf-core:master with commit 6cd5243 Sep 11, 2026
25 checks passed
ramprasadn pushed a commit to ramprasadn/modules that referenced this pull request Sep 11, 2026
* feat: add modkit/extract/full module

Add new nf-core module wrapping `modkit extract full`, which transforms
the MM/ML tags in a modBAM into a tab-separated per-read-per-position
probability table. Output can be BGZF-compressed via `--bgzf` in
`ext.args`. Useful for downstream custom filtering, plotting, and ML
training on read-level methylation probabilities.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor(modkit/extractfull): rename from modkit/extract/full

nf-core module names must be <tool> or <tool>/<subtool> (two levels max);
`modkit/extract/full` failed the `main_nf_module_granularity` lint check.

Renamed directory, process name (MODKIT_EXTRACTFULL), meta.yml name and
nf-test tags accordingly.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01E7RSvpouxbMBdimYSwJ5ZP

* fix(modkit/extractfull): sanitizeOutput snapshots + bump ont-modkit to 0.6.4

Ports the review changes from nf-core#11286 to this module.

- Snapshot via `sanitizeOutput(process.out)` with `assert process.success`
  outside `assertAll`, so a tool failure fails fast rather than reporting a
  snapshot mismatch against empty output. Requires nft-utils 1.x, which is
  why master is merged in first.
- Drop the redundant exists/size asserts on the non-stub test; the snapshot
  already covers the tsv and an empty file is caught by the md5 lint check.
- Remove tests/nextflow.config (it only set an empty ext.args) and the
  `config` directive that loaded it.
- Bump ont-modkit 0.6.1 -> 0.6.4 (clears the `bioconda_latest` lint warning);
  container tag `0.6.4--h7f49ad2_0` verified on the Galaxy depot and quay.io.

Non-stub tsv md5 is unchanged at 8300986c98d1b67973439ced4e7f233c across both
the config removal and the version bump.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JAsT7dzCcLSicJxFRWJozr

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: Famke Bäuerle <45968370+famosab@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting-changes will be closed after 30 days

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants