From 79b261f33b0be8e61bf3be5bfa0ec66690770945 Mon Sep 17 00:00:00 2001 From: Jukka Kurkela Date: Tue, 1 Jan 2019 12:35:02 +0200 Subject: [PATCH 1/9] increase spacing until there is a range --- src/scales/scale.linearbase.js | 10 +++++++--- test/specs/scale.linear.tests.js | 25 +++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/src/scales/scale.linearbase.js b/src/scales/scale.linearbase.js index 808f454407a..b4ad762f843 100644 --- a/src/scales/scale.linearbase.js +++ b/src/scales/scale.linearbase.js @@ -24,6 +24,7 @@ function generateTicks(generationOptions, dataRange) { var max = generationOptions.max; var precision = generationOptions.precision; var spacing, factor, niceMin, niceMax, numSpaces; + var i = 0; // spacing is set to a nice number of the dataRange divided by maxNumSpaces. // stepSize is used as a minimum unit if it is specified. @@ -42,9 +43,12 @@ function generateTicks(generationOptions, dataRange) { factor = Math.pow(10, precision); spacing = Math.ceil(spacing * factor) / factor; } - - niceMin = Math.floor(dataRange.min / spacing) * spacing; - niceMax = Math.ceil(dataRange.max / spacing) * spacing; + do { + niceMin = Math.floor(dataRange.min / spacing) * spacing; + niceMax = Math.ceil(dataRange.max / spacing) * spacing; + spacing *= 2; + } while (niceMin === niceMax && ++i < 8); + spacing /= 2; // If min, max and stepSize is set and they make an evenly spaced scale use it. if (stepSize) { diff --git a/test/specs/scale.linear.tests.js b/test/specs/scale.linear.tests.js index a186c6cc7fe..e19b9d79182 100644 --- a/test/specs/scale.linear.tests.js +++ b/test/specs/scale.linear.tests.js @@ -1170,4 +1170,29 @@ describe('Linear Scale', function() { expect(data[0]._model.base + minBarLength).toEqual(data[0]._model.x); expect(data[1]._model.base - minBarLength).toEqual(data[1]._model.x); }); + + it('Should ensure that the scale has a max and min that are not equal when data contains values that are very close to each other', function() { + var chart = window.acquireChart({ + type: 'scatter', + data: { + datasets: [{ + data: [ + {x: 1, y: 1.8548483304974972}, + {x: 2, y: 1.8548483304974974}, + ] + }], + }, + options: { + scales: { + yAxes: [{ + id: 'yScale0', + type: 'linear', + }] + } + } + }); + + expect(chart.scales.yScale0).not.toEqual(undefined); // must construct + expect(chart.scales.yScale0.max).toBeGreaterThan(chart.scales.yScale0.min); + }); }); From fe1c1f3a673445ef74e123ef17b9b2d629cfc572 Mon Sep 17 00:00:00 2001 From: Jukka Kurkela Date: Wed, 2 Jan 2019 00:38:01 +0200 Subject: [PATCH 2/9] add comment --- src/scales/scale.linearbase.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/scales/scale.linearbase.js b/src/scales/scale.linearbase.js index b4ad762f843..07f3ab9a275 100644 --- a/src/scales/scale.linearbase.js +++ b/src/scales/scale.linearbase.js @@ -43,6 +43,10 @@ function generateTicks(generationOptions, dataRange) { factor = Math.pow(10, precision); spacing = Math.ceil(spacing * factor) / factor; } + + // Loop here is for the case when min / spacing === max / spacing + // This happens due to float inccuracy when min and max are really close to each other + // One case: min = 1.8548483304974972 and max = 1.8548483304974974 do { niceMin = Math.floor(dataRange.min / spacing) * spacing; niceMax = Math.ceil(dataRange.max / spacing) * spacing; From 6a43dfa1a5cb34bf1cc8b217cc942791ecf53aec Mon Sep 17 00:00:00 2001 From: Jukka Kurkela Date: Thu, 3 Jan 2019 18:38:27 +0200 Subject: [PATCH 3/9] extra newline removed --- src/scales/scale.linearbase.js | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/scales/scale.linearbase.js b/src/scales/scale.linearbase.js index 07f3ab9a275..9485f9496a1 100644 --- a/src/scales/scale.linearbase.js +++ b/src/scales/scale.linearbase.js @@ -24,7 +24,6 @@ function generateTicks(generationOptions, dataRange) { var max = generationOptions.max; var precision = generationOptions.precision; var spacing, factor, niceMin, niceMax, numSpaces; - var i = 0; // spacing is set to a nice number of the dataRange divided by maxNumSpaces. // stepSize is used as a minimum unit if it is specified. @@ -44,15 +43,15 @@ function generateTicks(generationOptions, dataRange) { spacing = Math.ceil(spacing * factor) / factor; } - // Loop here is for the case when min / spacing === max / spacing - // This happens due to float inccuracy when min and max are really close to each other - // One case: min = 1.8548483304974972 and max = 1.8548483304974974 - do { - niceMin = Math.floor(dataRange.min / spacing) * spacing; - niceMax = Math.ceil(dataRange.max / spacing) * spacing; - spacing *= 2; - } while (niceMin === niceMax && ++i < 8); - spacing /= 2; + niceMin = Math.floor(dataRange.min / spacing) * spacing; + niceMax = Math.ceil(dataRange.max / spacing) * spacing; + + if (niceMin === niceMax) { + // This happens due to float inccuracy when min and max are really close to each other + // One case: min = 1.8548483304974972 and max = 1.8548483304974974 + ticks.push(niceMin - helpers.EPSILON, niceMax, niceMax + helpers.EPSILON); + return ticks; + } // If min, max and stepSize is set and they make an evenly spaced scale use it. if (stepSize) { From f6c9fa75830d441ca97cef1eb81c5b80044cbac3 Mon Sep 17 00:00:00 2001 From: Jukka Kurkela Date: Thu, 3 Jan 2019 21:12:39 +0200 Subject: [PATCH 4/9] 2 points is enough --- src/scales/scale.linearbase.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/scales/scale.linearbase.js b/src/scales/scale.linearbase.js index 9485f9496a1..b2fe3c98aae 100644 --- a/src/scales/scale.linearbase.js +++ b/src/scales/scale.linearbase.js @@ -49,7 +49,7 @@ function generateTicks(generationOptions, dataRange) { if (niceMin === niceMax) { // This happens due to float inccuracy when min and max are really close to each other // One case: min = 1.8548483304974972 and max = 1.8548483304974974 - ticks.push(niceMin - helpers.EPSILON, niceMax, niceMax + helpers.EPSILON); + ticks.push(dataRange.min, dataRange.max); return ticks; } From e70c33436f86c02142eb60f99d2c8f8e26539d3f Mon Sep 17 00:00:00 2001 From: Jukka Kurkela Date: Thu, 3 Jan 2019 21:24:06 +0200 Subject: [PATCH 5/9] save a line --- src/scales/scale.linearbase.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/scales/scale.linearbase.js b/src/scales/scale.linearbase.js index b2fe3c98aae..ba314039caa 100644 --- a/src/scales/scale.linearbase.js +++ b/src/scales/scale.linearbase.js @@ -49,8 +49,7 @@ function generateTicks(generationOptions, dataRange) { if (niceMin === niceMax) { // This happens due to float inccuracy when min and max are really close to each other // One case: min = 1.8548483304974972 and max = 1.8548483304974974 - ticks.push(dataRange.min, dataRange.max); - return ticks; + return [dataRange.min, dataRange.max]; } // If min, max and stepSize is set and they make an evenly spaced scale use it. From 48314ed80e5f3ab072e45418e8daae5a84837eec Mon Sep 17 00:00:00 2001 From: Jukka Kurkela Date: Thu, 3 Jan 2019 22:45:01 +0200 Subject: [PATCH 6/9] another solution --- src/scales/scale.linearbase.js | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/scales/scale.linearbase.js b/src/scales/scale.linearbase.js index ba314039caa..237a11e1723 100644 --- a/src/scales/scale.linearbase.js +++ b/src/scales/scale.linearbase.js @@ -17,6 +17,7 @@ function generateTicks(generationOptions, dataRange) { // "nice number" algorithm. See https://stackoverflow.com/questions/8506881/nice-label-algorithm-for-charts-with-minimum-ticks // for details. + var MAX_PRECISION = 1e14; var stepSize = generationOptions.stepSize; var unit = stepSize || 1; var maxNumSpaces = generationOptions.maxTicks - 1; @@ -25,6 +26,10 @@ function generateTicks(generationOptions, dataRange) { var precision = generationOptions.precision; var spacing, factor, niceMin, niceMax, numSpaces; + // sanitize dataRangee to MAX_PRECISION + dataRange.min = Math.floor(dataRange.min * MAX_PRECISION) / MAX_PRECISION; + dataRange.max = Math.ceil(dataRange.max * MAX_PRECISION) / MAX_PRECISION; + // spacing is set to a nice number of the dataRange divided by maxNumSpaces. // stepSize is used as a minimum unit if it is specified. spacing = helpers.niceNum((dataRange.max - dataRange.min) / maxNumSpaces / unit) * unit; @@ -46,12 +51,6 @@ function generateTicks(generationOptions, dataRange) { niceMin = Math.floor(dataRange.min / spacing) * spacing; niceMax = Math.ceil(dataRange.max / spacing) * spacing; - if (niceMin === niceMax) { - // This happens due to float inccuracy when min and max are really close to each other - // One case: min = 1.8548483304974972 and max = 1.8548483304974974 - return [dataRange.min, dataRange.max]; - } - // If min, max and stepSize is set and they make an evenly spaced scale use it. if (stepSize) { // If very close to our whole number, use it. From dbfe1b5ef1782c9d2b61cca031ba443699ce6743 Mon Sep 17 00:00:00 2001 From: Jukka Kurkela Date: Thu, 3 Jan 2019 23:12:53 +0200 Subject: [PATCH 7/9] use variables instead of changing dataRange,apply a lower bound to spacing --- src/scales/scale.linearbase.js | 15 ++++++++------- test/specs/scale.linear.tests.js | 2 +- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/scales/scale.linearbase.js b/src/scales/scale.linearbase.js index 237a11e1723..568a67535b4 100644 --- a/src/scales/scale.linearbase.js +++ b/src/scales/scale.linearbase.js @@ -18,6 +18,7 @@ function generateTicks(generationOptions, dataRange) { // for details. var MAX_PRECISION = 1e14; + var MIN_SPACING = 2e-15; var stepSize = generationOptions.stepSize; var unit = stepSize || 1; var maxNumSpaces = generationOptions.maxTicks - 1; @@ -27,13 +28,14 @@ function generateTicks(generationOptions, dataRange) { var spacing, factor, niceMin, niceMax, numSpaces; // sanitize dataRangee to MAX_PRECISION - dataRange.min = Math.floor(dataRange.min * MAX_PRECISION) / MAX_PRECISION; - dataRange.max = Math.ceil(dataRange.max * MAX_PRECISION) / MAX_PRECISION; + var rmin = Math.floor(dataRange.min * MAX_PRECISION) / MAX_PRECISION; + var rmax = Math.ceil(dataRange.max * MAX_PRECISION) / MAX_PRECISION; // spacing is set to a nice number of the dataRange divided by maxNumSpaces. // stepSize is used as a minimum unit if it is specified. - spacing = helpers.niceNum((dataRange.max - dataRange.min) / maxNumSpaces / unit) * unit; - numSpaces = Math.ceil(dataRange.max / spacing) - Math.floor(dataRange.min / spacing); + // use MIN_SPACING as lower bound to avoid uneven steps with tiny numbers + spacing = Math.max(helpers.niceNum((rmax - rmin) / maxNumSpaces / unit) * unit, MIN_SPACING); + numSpaces = Math.ceil(rmax / spacing) - Math.floor(rmin / spacing); if (numSpaces > maxNumSpaces) { // If the calculated num of spaces exceeds maxNumSpaces, recalculate it spacing = helpers.niceNum(numSpaces * spacing / maxNumSpaces / unit) * unit; @@ -48,8 +50,8 @@ function generateTicks(generationOptions, dataRange) { spacing = Math.ceil(spacing * factor) / factor; } - niceMin = Math.floor(dataRange.min / spacing) * spacing; - niceMax = Math.ceil(dataRange.max / spacing) * spacing; + niceMin = Math.floor(rmin / spacing) * spacing; + niceMax = Math.ceil(rmax / spacing) * spacing; // If min, max and stepSize is set and they make an evenly spaced scale use it. if (stepSize) { @@ -69,7 +71,6 @@ function generateTicks(generationOptions, dataRange) { } else { numSpaces = Math.ceil(numSpaces); } - niceMin = Math.round(niceMin * factor) / factor; niceMax = Math.round(niceMax * factor) / factor; ticks.push(helpers.isNullOrUndef(min) ? niceMin : min); diff --git a/test/specs/scale.linear.tests.js b/test/specs/scale.linear.tests.js index e19b9d79182..bdf8a9386f9 100644 --- a/test/specs/scale.linear.tests.js +++ b/test/specs/scale.linear.tests.js @@ -1171,7 +1171,7 @@ describe('Linear Scale', function() { expect(data[1]._model.base - minBarLength).toEqual(data[1]._model.x); }); - it('Should ensure that the scale has a max and min that are not equal when data contains values that are very close to each other', function() { + it('Should generate max and min that are not equal when data contains values that are very close to each other', function() { var chart = window.acquireChart({ type: 'scatter', data: { From 363c295c79ca4621a79abecc92cb1530c47ef6bd Mon Sep 17 00:00:00 2001 From: Jukka Kurkela Date: Fri, 4 Jan 2019 15:17:33 +0200 Subject: [PATCH 8/9] spacing < MIN_SPACING --- src/scales/scale.linearbase.js | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/src/scales/scale.linearbase.js b/src/scales/scale.linearbase.js index 568a67535b4..d504d54b568 100644 --- a/src/scales/scale.linearbase.js +++ b/src/scales/scale.linearbase.js @@ -17,8 +17,8 @@ function generateTicks(generationOptions, dataRange) { // "nice number" algorithm. See https://stackoverflow.com/questions/8506881/nice-label-algorithm-for-charts-with-minimum-ticks // for details. - var MAX_PRECISION = 1e14; - var MIN_SPACING = 2e-15; + // Minimum spacing between ticks + var MIN_SPACING = 1e-14; var stepSize = generationOptions.stepSize; var unit = stepSize || 1; var maxNumSpaces = generationOptions.maxTicks - 1; @@ -27,21 +27,26 @@ function generateTicks(generationOptions, dataRange) { var precision = generationOptions.precision; var spacing, factor, niceMin, niceMax, numSpaces; - // sanitize dataRangee to MAX_PRECISION - var rmin = Math.floor(dataRange.min * MAX_PRECISION) / MAX_PRECISION; - var rmax = Math.ceil(dataRange.max * MAX_PRECISION) / MAX_PRECISION; + var rmin = dataRange.min; + var rmax = dataRange.max; + var isNullOrUndef = helpers.isNullOrUndef; // spacing is set to a nice number of the dataRange divided by maxNumSpaces. // stepSize is used as a minimum unit if it is specified. - // use MIN_SPACING as lower bound to avoid uneven steps with tiny numbers - spacing = Math.max(helpers.niceNum((rmax - rmin) / maxNumSpaces / unit) * unit, MIN_SPACING); + spacing = helpers.niceNum((rmax - rmin) / maxNumSpaces / unit) * unit; + + // In case of really small numbers and min / max are undefined, default to rmin / rmax + if (spacing < MIN_SPACING && isNullOrUndef(min) && isNullOrUndef(max)) { + return [rmin, rmax]; + } + numSpaces = Math.ceil(rmax / spacing) - Math.floor(rmin / spacing); if (numSpaces > maxNumSpaces) { // If the calculated num of spaces exceeds maxNumSpaces, recalculate it spacing = helpers.niceNum(numSpaces * spacing / maxNumSpaces / unit) * unit; } - if (stepSize || helpers.isNullOrUndef(precision)) { + if (stepSize || isNullOrUndef(precision)) { // If a precision is not specified, calculate factor based on spacing factor = Math.pow(10, helpers.decimalPlaces(spacing)); } else { @@ -56,10 +61,10 @@ function generateTicks(generationOptions, dataRange) { // If min, max and stepSize is set and they make an evenly spaced scale use it. if (stepSize) { // If very close to our whole number, use it. - if (!helpers.isNullOrUndef(min) && helpers.almostWhole(min / spacing, spacing / 1000)) { + if (!isNullOrUndef(min) && helpers.almostWhole(min / spacing, spacing / 1000)) { niceMin = min; } - if (!helpers.isNullOrUndef(max) && helpers.almostWhole(max / spacing, spacing / 1000)) { + if (!isNullOrUndef(max) && helpers.almostWhole(max / spacing, spacing / 1000)) { niceMax = max; } } @@ -71,13 +76,14 @@ function generateTicks(generationOptions, dataRange) { } else { numSpaces = Math.ceil(numSpaces); } + niceMin = Math.round(niceMin * factor) / factor; niceMax = Math.round(niceMax * factor) / factor; - ticks.push(helpers.isNullOrUndef(min) ? niceMin : min); + ticks.push(isNullOrUndef(min) ? niceMin : min); for (var j = 1; j < numSpaces; ++j) { ticks.push(Math.round((niceMin + j * spacing) * factor) / factor); } - ticks.push(helpers.isNullOrUndef(max) ? niceMax : max); + ticks.push(isNullOrUndef(max) ? niceMax : max); return ticks; } From ef226c764b9e407fc8511150e6a89a8e0c6dd5a8 Mon Sep 17 00:00:00 2001 From: Jukka Kurkela Date: Sun, 6 Jan 2019 19:00:05 +0200 Subject: [PATCH 9/9] requested changes --- src/scales/scale.linearbase.js | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/src/scales/scale.linearbase.js b/src/scales/scale.linearbase.js index d504d54b568..057e68ffc3d 100644 --- a/src/scales/scale.linearbase.js +++ b/src/scales/scale.linearbase.js @@ -4,6 +4,7 @@ var helpers = require('../helpers/index'); var Scale = require('../core/core.scale'); var noop = helpers.noop; +var isNullOrUndef = helpers.isNullOrUndef; /** * Generate a set of linear ticks @@ -17,7 +18,6 @@ function generateTicks(generationOptions, dataRange) { // "nice number" algorithm. See https://stackoverflow.com/questions/8506881/nice-label-algorithm-for-charts-with-minimum-ticks // for details. - // Minimum spacing between ticks var MIN_SPACING = 1e-14; var stepSize = generationOptions.stepSize; var unit = stepSize || 1; @@ -25,17 +25,13 @@ function generateTicks(generationOptions, dataRange) { var min = generationOptions.min; var max = generationOptions.max; var precision = generationOptions.precision; - var spacing, factor, niceMin, niceMax, numSpaces; - var rmin = dataRange.min; var rmax = dataRange.max; - var isNullOrUndef = helpers.isNullOrUndef; - - // spacing is set to a nice number of the dataRange divided by maxNumSpaces. - // stepSize is used as a minimum unit if it is specified. - spacing = helpers.niceNum((rmax - rmin) / maxNumSpaces / unit) * unit; + var spacing = helpers.niceNum((rmax - rmin) / maxNumSpaces / unit) * unit; + var factor, niceMin, niceMax, numSpaces; - // In case of really small numbers and min / max are undefined, default to rmin / rmax + // Beyond MIN_SPACING floating point numbers being to lose precision + // such that we can't do the math necessary to generate ticks if (spacing < MIN_SPACING && isNullOrUndef(min) && isNullOrUndef(max)) { return [rmin, rmax]; }