-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Histogram events & bin hover label improvements #2113
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
Changes from 3 commits
59b6463
6dc9904
ccf1a76
1b356d2
8f8add1
7686096
8d36d81
a7e2f32
6b74fbe
b91d1cb
ad0e08f
6cb4e71
7581567
51ad8c2
3aa03f7
cc454dc
2696c1c
0331a16
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,11 @@ | |
var isNumeric = require('fast-isnumeric'); | ||
var loggers = require('./loggers'); | ||
|
||
// don't trust floating point equality - fraction of bin size to call | ||
// "on the line" and ensure that they go the right way specified by | ||
// linelow | ||
var roundingError = 1e-9; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm a little surprised that this hadn't come up before (other than the test below that was actually wrong before), and obviously it's a bit questionable exactly what small fraction to put here... but if we just use straight |
||
|
||
|
||
/** | ||
* findBin - find the bin for val - note that it can return outside the | ||
|
@@ -26,20 +31,21 @@ var loggers = require('./loggers'); | |
exports.findBin = function(val, bins, linelow) { | ||
if(isNumeric(bins.start)) { | ||
return linelow ? | ||
Math.ceil((val - bins.start) / bins.size) - 1 : | ||
Math.floor((val - bins.start) / bins.size); | ||
Math.ceil((val - bins.start) / bins.size - roundingError) - 1 : | ||
Math.floor((val - bins.start) / bins.size + roundingError); | ||
} | ||
else { | ||
var n1 = 0, | ||
n2 = bins.length, | ||
c = 0, | ||
n, | ||
test; | ||
if(bins[bins.length - 1] >= bins[0]) { | ||
var n1 = 0; | ||
var n2 = bins.length; | ||
var c = 0; | ||
var binSize = (n2 > 1) ? (bins[n2 - 1] - bins[0]) / (n2 - 1) : 1; | ||
var n, test; | ||
if(binSize >= 0) { | ||
test = linelow ? lessThan : lessOrEqual; | ||
} else { | ||
test = linelow ? greaterOrEqual : greaterThan; | ||
} | ||
val += binSize * roundingError * (linelow ? -1 : 1) * (binSize >= 0 ? 1 : -1); | ||
// c is just to avoid infinite loops if there's an error | ||
while(n1 < n2 && c++ < 100) { | ||
n = Math.floor((n1 + n2) / 2); | ||
|
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.
@etpinard I made
getDimText
in parallel with you addingd.(x|y)Label
andAxes.hoverLabelText
- I suppose in principle we could used.(x|y)Label
for this too instead of addingd.(x|y)LabelVal(0|1)
, maybe that would actually be simpler... let me take a look.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.
yeah, better to use
d.(x|x)Label
-> 51ad8c2There 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.
Much better in 51ad8c2
Thanks!