Skip Infinity datapoints in getRightValue #3133
Conversation
|
+1 to merge |
|
@bcongdon It would be good to write a test for this case as well |
|
I added |
|
Looks good. Thanks @bcongdon The CI error looks like its from Coveralls again, so not to worry about it :) |
src/core/core.scale.js
Outdated
| getRightValue: function(rawValue) { | ||
| // Null and undefined values first | ||
| if (rawValue === null || typeof(rawValue) === 'undefined') { | ||
| if (rawValue === null || typeof(rawValue) === 'undefined' || rawValue === Infinity) { |
There was a problem hiding this comment.
FWIW, we would have to test -Infinity as well.
I would use the isFinite global function [1] (/!\ not the Number.isFinite function, which IE doesn't support [2]). The check would need to be after the "object" checks, though.
[1] https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/isFinite
[2] https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/isFinite
|
@MatthieuRivaud: Switched the logic to use isFinite() |
|
@simonbrunel @zachpanz88 can we merge this? |
|
Looks good to me, merge conflicts should get fixed first |
src/core/core.scale.js
Outdated
| // isNaN(object) returns true, so make sure NaN is checking for a number | ||
| if (typeof(rawValue) === 'number' && isNaN(rawValue)) { | ||
| // isNaN(object) returns true, so make sure NaN is checking for a number; Discard Infinite values | ||
| if (typeof(rawValue) === 'number' && (isNaN(rawValue) || !isFinite(rawValue))) { |
There was a problem hiding this comment.
Doesn't isFinite include !isNaN?
According MDN:
Returns false if the argument is positive or negative Infinity or NaN; otherwise, true.
So could be simply if (typeof(rawValue) === 'number' && !isFinite(rawValue)) {, right?
|
Merged in 62ef40e |
5 participants
Addresses #3125.
Skips
Infinitydatapoints ingetRightValueto fix graph creation glitches resulting from 'infinite' datapoints.