Skip to content

Allow N#597

Open
killercup wants to merge 1 commit intonanoporetech:masterfrom
Softleif:allow-n
Open

Allow N#597
killercup wants to merge 1 commit intonanoporetech:masterfrom
Softleif:allow-n

Conversation

@killercup
Copy link
Contributor

We were testing with a BAM file where reads have Ns and modkit crashed on the unreachable! in modkit-core/src/pileup/base_mods_adapter.rs:77. I think this "fixes" this by ignoring this position. I also tried to come up with some other places this could be problematic and then also tried to have Claude write tests that exercise this.

@ArtRand
Copy link
Contributor

ArtRand commented Mar 19, 2026

Hello @killercup,

Out of curiosity, where are you getting sequencing reads with N bases?

Thanks for the contribution, I'll take a look when I start rolling up all the changes for v0.6.2. One thing of note, this code is part of the hottest most critical loop in pileup, so I'll have to make sure there isn't any performance regression. From a quick glance, it should be fine, but I'll need to check. If you have a samply trace from the released version and this one that would save me some time.

@killercup
Copy link
Contributor Author

Thanks for the quick reply! We're working on https://bitbucket.org/bsblabludwig/rastair which works with TAPS reads and can write modBAM.

Good point about the performance, I can compare this for you. (Might have to wait until tomorrow though)

@ArtRand
Copy link
Contributor

ArtRand commented Mar 19, 2026

Yes I saw Rastair, looks like a nice project! No rush on the perf data. Thanks for the contribution.

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