-
-
Notifications
You must be signed in to change notification settings - Fork 348
Align heatmap RNASeq scores with oncoprint tracks #11592 #5225
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
base: master
Are you sure you want to change the base?
Conversation
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.
Thanks! This def looks better!
Some comment in the description as well how the aggregation logic would be good. Also ideally both tracks should use the same functions for the aggregation logic, I didn't confirm in code if that's how it works
Note that selecting the patient level aggregation logic for RNASeq is a bit more complex than for mutation data (union), but this seems reasonable (if an expression query with threshold exists -> pick thresholded samples absolute max value)
| query | ||
| )!.data!; | ||
|
|
||
| // get Z-score threshold from the store (default to 2 if not present) |
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.
can you see if there is a constand somewhere for default 2?
| default: | ||
| bestValue = _.maxBy(dataWithValue, (d: HeatmapCaseDatum) => | ||
| Math.abs(d.value) | ||
|
|
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.
would be good to have a unit test for this. let me know if you need help setting that up
Fix frontend ticket #11592
Note: left original logic as fallback in DataUtils.ts