Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix bug with incorrect coverage being rendered in jbrowse #105

Merged
merged 6 commits into from
Nov 11, 2024

Conversation

cmdcolin
Copy link
Collaborator

@cmdcolin cmdcolin commented Nov 11, 2024

I was browsing jbrowse as one does and found a location in a BAM file where the coverage was displayed wrong, approximately doubled, and at "static block" (the jbrowse-2-ism, not bam file format-ism) boundaries

i found that older versions did not have the bug, and bisected to a diff here in jbrowse 2 https://github.com/gmod/jbrowse-components/compare/82618939977da5e71a419~1..82618939977da5e71a419 which led to it being between v1.1.8..v2.0.0 in bam-js https://github.com/gmod/bam-js/compare/v1.1.18..v2.0.0

the cause is fairly technical but a refactoring oversight, where a new "chunk object" (bam file format-ism, representing a range in the file) was not constructed,

the diff was just this to fix

@@ -185,11 +183,10 @@ export default class BAI extends IndexFile {
     // Find chunks in overlapping bins.  Leaf bins (< 4681) are not pruned
     for (const [start, end] of overlappingBins) {
       for (let bin = start; bin <= end; bin++) {
         if (ba.binIndex[bin]) {
           const binChunks = ba.binIndex[bin]
           for (const binChunk of binChunks) {
-            chunks.push(binChunk)
+            chunks.push(new Chunk(binChunk.minv, binChunk.maxv, bin))
           }
         }
       }

my guess is that this bug was somewhat rare but did happen

screenshots

after
image

before
image

@cmdcolin cmdcolin merged commit 1cdb0dd into master Nov 11, 2024
2 checks passed
@cmdcolin cmdcolin deleted the backtoworking branch November 11, 2024 06:15
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.

1 participant