-
Notifications
You must be signed in to change notification settings - Fork 2
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
Update coverage tracking #14
base: master
Are you sure you want to change the base?
Conversation
…and Genotype.js constructors to take in objects instead
…atureDataSource-test.js file
…th RemoteRequest file
…and Genotype.js constructors to take in objects instead
fixed no data error for alignments and max rows for PileupTrack
…uest areas already in cache.
Variant fixes
# Conflicts: # src/main/sources/GA4GHDataSource.js
Update Feature Transparency Based on Score
Can one of the admins verify this patch? |
src/main/viz/PileupCoverageTrack.js
Outdated
localYScaleForRef(ref: string): (y: number) => number { | ||
var maxCoverage = this.cache.maxCoverageForRef(ref); | ||
|
||
var padding = 20; // TODO: move into style |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is the TODO for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an artifact of the original yScaleForRef.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need the todo then?
src/main/viz/PileupCoverageTrack.js
Outdated
@@ -65,11 +65,21 @@ class CoverageTiledCanvas extends TiledCanvas { | |||
.nice(); | |||
} | |||
|
|||
localYScaleForRef(ref: string): (y: number) => number { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is it called local?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The padding created in the original yScaleForRef seems to cause the y-coordinate of the DataCanvasRenderingContext2D in the render function to be shifted off by 10 degrees. The original yScaleForRef is still used to generate the tick marks, so I create another one for rendering coverage tiles that shouldn't be used outside of context of coverage tiles.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I dont love the idea of having YScaleForRef and localYScaleForRef. We should probably just have 1 function.
d845a3a
to
95fa18e
Compare
95fa18e
to
cedf37a
Compare
Updated |
These changes should go in the hammerlab pileup |
Before

After

Modify coverage track to show lower coverage regions.