Skip to content

Add module: modkit/localize - #11120

Open
sahuno wants to merge 3 commits into
nf-core:masterfrom
sahuno:add-modkit-localize
Open

Add module: modkit/localize#11120
sahuno wants to merge 3 commits into
nf-core:masterfrom
sahuno:add-modkit-localize

Conversation

@sahuno

@sahuno sahuno commented Apr 5, 2026

Copy link
Copy Markdown
Contributor

New module: modkit/localize

Tool

modkit — A bioinformatics tool for working with modified bases from Oxford Nanopore sequencing data.

What this module does

MODKIT_LOCALIZE aggregates bedMethyl pileup counts around genomic features of interest (e.g. CpG islands, gene bodies, repeat elements), producing:

  • A TSV of percent-modification vs. offset from feature midpoints
  • An optional interactive HTML chart

This enables investigation of modification enrichment or depletion patterns relative to genomic landmarks, analogous to a methylation profile around features.

Inputs

  • bgzip-compressed + tabix-indexed bedMethyl file (from modkit pileup)
  • Genome sizes file (e.g. .fai or .sizes)
  • BED file of regions to localize around

Outputs

  • *.tsv — percent-modification vs. offset table
  • *.html — interactive chart
  • *.log — debug log

Tests

Tests chain MODKIT_PILEUPTABIX_TABIXMODKIT_LOCALIZE using existing nf-core test-datasets (homo_sapiens nanopore BAM + genome.fasta.fai). Regions BED is created inline — no new test data upload required.

  • nf-test passes locally (singularity profile)
  • nf-test passes on SLURM compute node (job 17362918, 2 tests PASSED in 13s)
  • nf-core modules lint passes: 48/48 tests, 0 warnings, 0 failures

Notes

  • --regions is required by the modkit 0.6.1 CLI (enforced at the tool level)
  • Container spec matches the existing modkit/pileup module
  • A companion modkit/localize/plot submodule for composite multi-sample visualization is planned as a follow-up PR

@sahuno
sahuno requested review from a team, edmundmiller and maxulysse as code owners April 5, 2026 01:47
@famosab

famosab commented Apr 10, 2026

Copy link
Copy Markdown
Contributor

We usually recommend to have one module per PR. That makes the review process easier and it is more likely that someone will review your PR. I would suggest to split this PR up into the dorado module and the modkit module :)

Adds `modkit localize`, which summarises methylation around a set of regions.

This PR previously carried three modules — `modkit/localize`,
`modkit/localize/plot` and the whole of `dorado/basecaller` — which is why
@famosab asked for it to be split. It now contains `modkit/localize` only:

- `dorado/basecaller` is dropped entirely; it is already in nf-core#11122, and the
  copy here was the older revision that still committed pod5 files into the
  repo.
- `modkit/localize/plot` moves to its own PR, renamed `modkit/localizeplot`
  since nf-core module paths are at most two levels deep.

Rebased onto current master, which required two fixes:

- `TABIX_TABIX` is now deprecated (its script is `assert false`, pointing at
  `HTSLIB/BGZIPTABIX`), and its input signature had grown from two elements to
  four. The test setup now uses `HTSLIB_BGZIPTABIX` to build the tabix index
  from the `MODKIT_PILEUP` output.
- `main.nf.test` never loaded `tests/nextflow.config`, so `params.module_args`
  never reached `ext.args` and the `--min-coverage 1 --window 1000` settings the
  test relies on were silently ignored. Added the `config` directive.

Also aligned the container declaration with the merged modkit modules
(`workflow.containerEngine in ['singularity', 'apptainer']` and the `quay.io/`
prefix required by lint).

nf-core/tools 4.1.0 lint: 59 passed, 0 warnings, 0 failed.
Both nf-tests pass under the singularity profile.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01E7RSvpouxbMBdimYSwJ5ZP
@sahuno
sahuno force-pushed the add-modkit-localize branch from 6079e68 to 58263ab Compare September 6, 2026 17:28
@sahuno

sahuno commented Sep 6, 2026

Copy link
Copy Markdown
Contributor Author

@famosab you were right, and sorry this took so long. Split done — this PR is
now modkit/localize only, six files, nothing else.

  • dorado/basecaller is gone from here entirely. It is already Add module: dorado/basecaller #11122, and the
    copy in this branch was the older revision that still committed pod5 files
    into the repo.
  • modkit/localize/plot moves to its own module, renamed modkit/localizeplot
    (three-level paths fail main_nf_module_granularity). I have not opened that
    PR yet — see the note at the bottom.

Rebasing onto current master surfaced two things that were quietly broken:

TABIX_TABIX is deprecated. Its script block on master is now
assert false with a message pointing at HTSLIB/BGZIPTABIX, and its input
signature had grown from two elements to four. The test setup now uses
HTSLIB_BGZIPTABIX to build the tabix index from the MODKIT_PILEUP output.

The test config was never loaded. main.nf.test had no config directive,
so params.module_args never reached ext.args and the
--min-coverage 1 --window 1000 values the test depends on were being silently
ignored. Added the directive.

Also aligned the container declaration with the merged modkit modules
(workflow.containerEngine in ['singularity', 'apptainer'], quay.io/ prefix).

nf-core/tools 4.1.0 lint: 59 passed, 0 warnings, 0 failed. Both nf-tests
pass under the singularity profile.

On modkit/localizeplot: it is ready except for its container, which currently
points at a temporary Wave build URI that is neither on the allowed registry
list nor stable. It needs a frozen community.wave.seqera.io/library/... image
built from its environment.yml. I would rather sort that out before opening the
PR than add another red one to the queue — but if you would prefer to see it now
regardless, say the word and I will open it.

sahuno and others added 2 commits September 9, 2026 14:21
….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 force-pushed the add-modkit-localize branch from 1d92bf8 to 369669c Compare September 9, 2026 18:31
@sahuno sahuno mentioned this pull request Sep 11, 2026
9 tasks
@sahuno
sahuno requested a review from famosab September 11, 2026 02:03
@sahuno

sahuno commented Sep 11, 2026

Copy link
Copy Markdown
Contributor Author

Update since the split: rebased onto master, tests moved to assert process.success + snapshot(sanitizeOutput(process.out)) (same pattern as #11286), snapshots regenerated, container bumped to ont-modkit 0.6.4. This PR is modkit/localize only — six files — CI is green and there are no open review threads. @famosab re-requesting review since you flagged the original bundle.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants