Skip to content

Exclude unmapped calls from CpG summaries - #650

Open
SuhasSrinivasan wants to merge 1 commit into
nanoporetech:masterfrom
SuhasSrinivasan:codex/fix-summary-cpg-unmapped
Open

Exclude unmapped calls from CpG summaries#650
SuhasSrinivasan wants to merge 1 commit into
nanoporetech:masterfrom
SuhasSrinivasan:codex/fix-summary-cpg-unmapped

Conversation

@SuhasSrinivasan

Copy link
Copy Markdown
Contributor

Fixes #649.

Summary

  • exclude unmapped records from indexed summary --cpg results, matching the documented --motif CG 0 behavior
  • add an end-to-end CLI regression with a valid unmapped MM/ML/MN record

Root cause and impact

The indexed summary path correctly constructs a CG motif for --cpg, but the later guard that skips unmapped-record collection checked only mapped_only, matched_only, and an explicit motif. Because the CpG shorthand flag was omitted, all unmapped modification calls were merged into the mapped reference-CpG histogram.

On the mapped-plus-unmapped fixture from #649, modkit 0.6.4 reports 20 reads and 186 C calls under --cpg, versus 10 reads and 77 calls under --motif CG 0. With this change, both forms report the same scientific counts.

Regression

The test copies the existing 10 mapped test records and appends an unmapped CCCC record with:

  • MM:Z:C+m?,0,0,0,0;
  • four ML values of 255
  • MN:i:4

It runs the actual CLI, normalizes TSV key/value ordering, and requires the mixed result to equal the mapped-only control with total_reads_used=10, count_reads_C=10, and C_total_mod_calls=77.

Validation

  • confirmed the finalized regression fails on the parent revision: total_reads_used is 11 instead of 10
  • cargo test -p modkit --test test_summary test_summary_cpg_excludes_unmapped_calls -- --exact
  • cargo test -p modkit --test test_summary (5 passed)
  • cargo test -p modkit --test test_pileup -- --test-threads=1 (16 passed, 10 ignored)
  • cargo test --workspace --quiet -- --test-threads=1
  • rustfmt --check --edition 2021 modkit-core/src/modbam_util/subcommands.rs modkit/tests/test_summary.rs
  • git diff --check
  • installed-versus-patched CLI comparison on the issue fixture: 20 / 186 becomes 10 / 77 and matches explicit --motif CG 0

An initial default-parallel package run exposed an existing integration-test collision: test_pileup_no_filt and test_pileup_with_header both write /tmp/test_pileup_nofilt.bed. The pileup target and full workspace pass when run serially; this branch does not touch pileup.

@SuhasSrinivasan
SuhasSrinivasan marked this pull request as ready for review August 5, 2026 05:56
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.

summary --cpg includes unmapped modification calls

1 participant