-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Fix coloring and hover data for nonuniform heatmap & contour bricks #2288
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
Conversation
src/traces/heatmap/plot.js
Outdated
function findInterpFromCenters(pixel, centerPixArray) { | ||
// if(pixel <= centerPixArray[0]) return {bin0: 0, bin1: 0, frac: 0}; | ||
var maxBin = centerPixArray.length - 1; | ||
// if(pixel >= centerPixArray[lastCenter]) return {bin0: lastCenter, bin1: lastCenter, frac: 0}; |
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.
Are these being temporarily commented out?
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.
Ah thanks - those are obsolete -> 3581c87
@alexcjohnson is that ok that we see Not relevant to this PR, but is that fine that hover labels appear at different positions than with |
I think so - it's certainly better than saying |
I believe this behaves as expected. For a heatmap, the hover label should not point to the data value but to the edge of the brick, whereas contour map labels should point exactly to the data value. Heatmaps you're defining, either implicitly or explicitly, bricks of a certain value, and you're saying "this is the value that applies to this brick". That metaphor gets a little bit muddied by smoothing the heatmap, but that's the idea anyway. Whereas with contour maps, you're implying that there's some continuous function and you've only sampled it at certain locations, the data points, but it has a distinct value at every (x,y) pair. But we don't extrapolate beyond the range of the data, even as far as the edge of the corresponding heatmap brick. |
Looks good then! 💃 |
Fixes #2233 - we were finding brick edges (as midpoints between data values) and then forever after using the midpoints of bricks as the hover data and (heatmap with
zsmooth: 'best'
or contour withcoloring: 'heatmap'
) the point of exact color matching, rather than using the original data values for these purposes.@dfcreative one more bugfix PR to review before I move onto building new stuff?