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 coloring and hover data for nonuniform heatmap & contour bricks #2288

Merged
merged 4 commits into from
Jan 24, 2018

Conversation

alexcjohnson
Copy link
Collaborator

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 with coloring: '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?

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};
Copy link
Contributor

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?

Copy link
Collaborator Author

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

@dy
Copy link
Contributor

dy commented Jan 24, 2018

@alexcjohnson is that ok that we see NaN in empty blocks?

image

Not relevant to this PR, but is that fine that hover labels appear at different positions than with contour? Ie. this is not possible with heatmap:
image

@alexcjohnson
Copy link
Collaborator Author

is that ok that we see NaN in empty blocks?

I think so - it's certainly better than saying 0. Might be nice to be able to customize what NaN displays as, but this is a reasonable default I think. See also #975

@alexcjohnson
Copy link
Collaborator Author

Not relevant to this PR, but is that fine that hover labels appear at different positions than with contour

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.

@dy
Copy link
Contributor

dy commented Jan 24, 2018

Looks good then! 💃

@alexcjohnson alexcjohnson merged commit 961d04b into master Jan 24, 2018
@alexcjohnson alexcjohnson deleted the nonuniform-bricks branch January 24, 2018 20:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Invalid heatmap values
2 participants