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 CRAM mismatches calculation regression in v2.2.0 #3342

Merged
merged 5 commits into from Nov 21, 2022

Conversation

cmdcolin
Copy link
Collaborator

@cmdcolin cmdcolin commented Nov 21, 2022

The PR at #3293 introduced a regression where CRAM mismatches would be calculated incorrectly with the 'i' readFeature type due to incorrectly assuming that readFeatures would have an indication of stretches of matches. This PR fixes that assumption using code similar to the CIGAR generation

@github-actions github-actions bot added the needs label triage Needs a label to show in changelog (breaking, enhancement, bug, documentation, or internal) label Nov 21, 2022
@cmdcolin cmdcolin added bug Something isn't working and removed needs label triage Needs a label to show in changelog (breaking, enhancement, bug, documentation, or internal) labels Nov 21, 2022
@github-actions github-actions bot added the needs label triage Needs a label to show in changelog (breaking, enhancement, bug, documentation, or internal) label Nov 21, 2022
@cmdcolin cmdcolin changed the title Fix CRAM mismatches calculation Fix CRAM mismatches calculation regression in v2.2.0 Nov 21, 2022
@codecov
Copy link

codecov bot commented Nov 21, 2022

Codecov Report

Merging #3342 (85b6117) into main (8a7c3bf) will decrease coverage by 0.02%.
The diff coverage is 77.68%.

❗ Current head 85b6117 differs from pull request most recent head f18ed93. Consider uploading reports for the commit f18ed93 to get more accurate results

@@            Coverage Diff             @@
##             main    #3342      +/-   ##
==========================================
- Coverage   59.16%   59.14%   -0.03%     
==========================================
  Files         750      751       +1     
  Lines       29266    29270       +4     
  Branches     7084     7083       -1     
==========================================
- Hits        17316    17312       -4     
- Misses      11756    11764       +8     
  Partials      194      194              
Impacted Files Coverage Δ
plugins/alignments/src/CramAdapter/util.ts 76.92% <76.92%> (ø)
...gnments/src/CramAdapter/CramSlightlyLazyFeature.ts 85.71% <100.00%> (+5.32%) ⬆️
...rative-view/src/ServerSideRenderedBlockContent.tsx 62.50% <0.00%> (-4.17%) ⬇️
...ns/alignments/src/PileupRenderer/PileupRenderer.ts 49.49% <0.00%> (-1.42%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@cmdcolin cmdcolin removed the needs label triage Needs a label to show in changelog (breaking, enhancement, bug, documentation, or internal) label Nov 21, 2022
@cmdcolin
Copy link
Collaborator Author

now does a "cross-check" by comparing the CIGAR and mismatches data structures from the same data file encoded in BAM and CRAM

this picked up a number of subtle logic bugs that were good to catch

@cmdcolin cmdcolin merged commit 4fef1dd into main Nov 21, 2022
@cmdcolin cmdcolin deleted the fix_cram_mismatches branch November 21, 2022 18:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant