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 crash displaying modifications called on softclipped regions of reads #2964

Merged
merged 5 commits into from
May 19, 2022

Conversation

cmdcolin
Copy link
Collaborator

Found a crash displaying modifications, example share link
https://jbrowse.org/code/jb2/v1.7.0/?config=test_data%2Fconfig_demo.json&session=share-Jfg7Adjvai&password=ZCjW1

This is probably due to displaying a modifications that was called on a soft clipped area of a read (e.g. a part of the read that did not properly map to the reference genome)

Instead of trying to display these (and currently, causing a crash) this PR proposes to filter them out

@github-actions github-actions bot added the needs label triage Needs a label to show in changelog (breaking, enhancement, bug, documentation, or internal) label May 19, 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 May 19, 2022
@codecov
Copy link

codecov bot commented May 19, 2022

Codecov Report

Merging #2964 (c6f9b84) into main (8b9d869) will increase coverage by 0.02%.
The diff coverage is 72.05%.

@@            Coverage Diff             @@
##             main    #2964      +/-   ##
==========================================
+ Coverage   60.67%   60.69%   +0.02%     
==========================================
  Files         601      601              
  Lines       27342    27361      +19     
  Branches     6654     6665      +11     
==========================================
+ Hits        16589    16606      +17     
- Misses      10442    10444       +2     
  Partials      311      311              
Impacted Files Coverage Δ
...lugins/alignments/src/LinearPileupDisplay/model.ts 64.97% <ø> (ø)
...s/alignments/src/PileupRenderer/PileupRenderer.tsx 47.91% <0.00%> (-0.29%) ⬇️
...ments/src/SNPCoverageAdapter/SNPCoverageAdapter.ts 54.90% <0.00%> (-1.48%) ⬇️
plugins/alignments/src/CramAdapter/CramAdapter.ts 83.94% <33.33%> (-0.62%) ⬇️
...lugins/alignments/src/BamAdapter/MismatchParser.ts 89.09% <91.66%> (-0.02%) ⬇️
plugins/alignments/src/BamAdapter/BamAdapter.ts 72.97% <100.00%> (ø)
...ments/src/LinearSNPCoverageDisplay/models/model.ts 63.15% <100.00%> (+0.39%) ⬆️
...ignments/src/PileupRenderer/PileupLayoutSession.ts 100.00% <100.00%> (ø)
products/jbrowse-web/src/util.ts 27.27% <0.00%> (ø)
packages/core/util/layouts/GranularRectLayout.ts 87.28% <0.00%> (+0.42%) ⬆️
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c6b42a7...c6f9b84. Read the comment docs.

@cmdcolin cmdcolin force-pushed the softclipped_modifications branch from fc6e895 to f94a72f Compare May 19, 2022 22:46
@cmdcolin cmdcolin force-pushed the softclipped_modifications branch from f94a72f to c6f9b84 Compare May 19, 2022 23:06
@cmdcolin
Copy link
Collaborator Author

This now, instead of filtering out negative coordinates in the softclipped bases, actually modifies the "getNextRefPos" to only return valid positions on the reference within the read, and so does not return areas in the softclipped bases.

This has the additional benefit of fixing a bug in getNextRefPos. This bug caused getNextRefPos to plot the wrong positions with large insertions from what I could tell. The new logic is simpler and uses some lessons learned from plotting reads without MD

@cmdcolin cmdcolin merged commit 4fbed2d into main May 19, 2022
@cmdcolin cmdcolin deleted the softclipped_modifications branch May 19, 2022 23:30
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.

1 participant