From 831ca6c932fb2b7223d6ead3ed746092b0f36014 Mon Sep 17 00:00:00 2001 From: archmoj Date: Sat, 2 Mar 2019 22:37:20 -0500 Subject: [PATCH 1/5] fix 3595 i.e. to avoid integer rounding errors in parcoords when using typed arrays --- src/traces/parcoords/defaults.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/traces/parcoords/defaults.js b/src/traces/parcoords/defaults.js index dfb4b7d3fe7..9eecba9d510 100644 --- a/src/traces/parcoords/defaults.js +++ b/src/traces/parcoords/defaults.js @@ -22,6 +22,11 @@ var mergeLength = require('./merge_length'); function handleLineDefaults(traceIn, traceOut, defaultColor, layout, coerce) { var lineColor = coerce('line.color', defaultColor); + if(!Array.isArray(lineColor) && Lib.isArrayOrTypedArray(lineColor)) { + // should convert typed arrays e.g. integers to real numbers + lineColor = traceOut.line.color = Array.prototype.slice.call(lineColor); + } + if(hasColorscale(traceIn, 'line') && Lib.isArrayOrTypedArray(lineColor)) { if(lineColor.length) { coerce('line.colorscale'); From 12baea7dce351385303febb729f83de8db7c668f Mon Sep 17 00:00:00 2001 From: archmoj Date: Sun, 3 Mar 2019 16:16:59 -0500 Subject: [PATCH 2/5] added a jasmine test to lock 3595 --- test/jasmine/tests/parcoords_test.js | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/test/jasmine/tests/parcoords_test.js b/test/jasmine/tests/parcoords_test.js index 2b42b71983b..09c1c72be8c 100644 --- a/test/jasmine/tests/parcoords_test.js +++ b/test/jasmine/tests/parcoords_test.js @@ -145,6 +145,28 @@ describe('parcoords initialization tests', function() { }); }); + it('\'line.color\' should convert typed arrays to normal arrays', function() { + var fullTrace = _supply({ + dimensions: [{ + range: [1, 5], + label: 'A', + values: [1, 4, 3] + }, { + range: [1, 5], + label: 'B', + values: [3, 1.5, 2], + }, { + range: [1, 5], + label: 'C', + values: [2, 4, 1], + }], + line: { + color: new Int32Array([0, 1, 2]) + } + }); + expect(Array.isArray(fullTrace.line.color) === true).toEqual(true); + }); + it('\'domain\' specification should have a default', function() { var fullTrace = _supply({}); expect(fullTrace.domain).toEqual({x: [0, 1], y: [0, 1]}); From e8cc760f642b932486638ef31a9cbf682926e06c Mon Sep 17 00:00:00 2001 From: archmoj Date: Sun, 3 Mar 2019 16:44:09 -0500 Subject: [PATCH 3/5] similar fix for parcoords.dimensions.values typedArray case --- src/traces/parcoords/defaults.js | 7 ++++++- test/jasmine/tests/parcoords_test.js | 8 +++++--- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/src/traces/parcoords/defaults.js b/src/traces/parcoords/defaults.js index 9eecba9d510..9a9c524d777 100644 --- a/src/traces/parcoords/defaults.js +++ b/src/traces/parcoords/defaults.js @@ -20,8 +20,8 @@ var maxDimensionCount = require('./constants').maxDimensionCount; var mergeLength = require('./merge_length'); function handleLineDefaults(traceIn, traceOut, defaultColor, layout, coerce) { - var lineColor = coerce('line.color', defaultColor); + var lineColor = coerce('line.color', defaultColor); if(!Array.isArray(lineColor) && Lib.isArrayOrTypedArray(lineColor)) { // should convert typed arrays e.g. integers to real numbers lineColor = traceOut.line.color = Array.prototype.slice.call(lineColor); @@ -49,6 +49,11 @@ function dimensionDefaults(dimensionIn, dimensionOut) { } var values = coerce('values'); + if(!Array.isArray(values) && Lib.isArrayOrTypedArray(values)) { + // should convert typed arrays e.g. integers to real numbers + values = dimensionOut.values = Array.prototype.slice.call(values); + } + var visible = coerce('visible'); if(!(values && values.length)) { visible = dimensionOut.visible = false; diff --git a/test/jasmine/tests/parcoords_test.js b/test/jasmine/tests/parcoords_test.js index 09c1c72be8c..7b19f6960c9 100644 --- a/test/jasmine/tests/parcoords_test.js +++ b/test/jasmine/tests/parcoords_test.js @@ -145,7 +145,7 @@ describe('parcoords initialization tests', function() { }); }); - it('\'line.color\' should convert typed arrays to normal arrays', function() { + it('\'dimensions.values\' and \'line.color\' should convert typed arrays to normal arrays', function() { var fullTrace = _supply({ dimensions: [{ range: [1, 5], @@ -154,17 +154,19 @@ describe('parcoords initialization tests', function() { }, { range: [1, 5], label: 'B', - values: [3, 1.5, 2], + values: new Float64Array([3, 1.5, 2]), }, { range: [1, 5], label: 'C', - values: [2, 4, 1], + values: new Int32Array([2, 4, 1]), }], line: { color: new Int32Array([0, 1, 2]) } }); expect(Array.isArray(fullTrace.line.color) === true).toEqual(true); + expect(Array.isArray(fullTrace.dimensions[1].values) === true).toEqual(true); + expect(Array.isArray(fullTrace.dimensions[2].values) === true).toEqual(true); }); it('\'domain\' specification should have a default', function() { From 30e2c259835e794ec2ca3a411618a5742fe27156 Mon Sep 17 00:00:00 2001 From: archmoj Date: Mon, 4 Mar 2019 11:00:24 -0500 Subject: [PATCH 4/5] moved convert typedArray logic from defaults to calc step --- src/traces/parcoords/calc.js | 16 +++++++++- src/traces/parcoords/defaults.js | 10 ------ test/jasmine/tests/parcoords_test.js | 48 ++++++++++++++-------------- 3 files changed, 39 insertions(+), 35 deletions(-) diff --git a/src/traces/parcoords/calc.js b/src/traces/parcoords/calc.js index 82ca943103c..7411de84a9b 100644 --- a/src/traces/parcoords/calc.js +++ b/src/traces/parcoords/calc.js @@ -14,7 +14,13 @@ var Lib = require('../../lib'); var wrap = require('../../lib/gup').wrap; module.exports = function calc(gd, trace) { - var cs = !!trace.line.colorscale && Lib.isArrayOrTypedArray(trace.line.color); + + for(var i = 0; i < trace.dimensions.length; i++) { + trace.dimensions[i].values = convertTypedArray(trace.dimensions[i].values); + } + trace.line.color = convertTypedArray(trace.line.color); + + var cs = !!trace.line.colorscale && Array.isArray(trace.line.color); var color = cs ? trace.line.color : constHalf(trace._length); var cscale = cs ? trace.line.colorscale : [[0, trace.line.color], [1, trace.line.color]]; @@ -39,3 +45,11 @@ function constHalf(len) { } return out; } + +function isTypedArray(a) { + return !Array.isArray(a) && Lib.isArrayOrTypedArray(a); +} + +function convertTypedArray(a) { + return (isTypedArray(a)) ? Array.prototype.slice.call(a) : a; +} diff --git a/src/traces/parcoords/defaults.js b/src/traces/parcoords/defaults.js index 9a9c524d777..dfb4b7d3fe7 100644 --- a/src/traces/parcoords/defaults.js +++ b/src/traces/parcoords/defaults.js @@ -20,12 +20,7 @@ var maxDimensionCount = require('./constants').maxDimensionCount; var mergeLength = require('./merge_length'); function handleLineDefaults(traceIn, traceOut, defaultColor, layout, coerce) { - var lineColor = coerce('line.color', defaultColor); - if(!Array.isArray(lineColor) && Lib.isArrayOrTypedArray(lineColor)) { - // should convert typed arrays e.g. integers to real numbers - lineColor = traceOut.line.color = Array.prototype.slice.call(lineColor); - } if(hasColorscale(traceIn, 'line') && Lib.isArrayOrTypedArray(lineColor)) { if(lineColor.length) { @@ -49,11 +44,6 @@ function dimensionDefaults(dimensionIn, dimensionOut) { } var values = coerce('values'); - if(!Array.isArray(values) && Lib.isArrayOrTypedArray(values)) { - // should convert typed arrays e.g. integers to real numbers - values = dimensionOut.values = Array.prototype.slice.call(values); - } - var visible = coerce('visible'); if(!(values && values.length)) { visible = dimensionOut.visible = false; diff --git a/test/jasmine/tests/parcoords_test.js b/test/jasmine/tests/parcoords_test.js index 7b19f6960c9..863fcd1a531 100644 --- a/test/jasmine/tests/parcoords_test.js +++ b/test/jasmine/tests/parcoords_test.js @@ -145,30 +145,6 @@ describe('parcoords initialization tests', function() { }); }); - it('\'dimensions.values\' and \'line.color\' should convert typed arrays to normal arrays', function() { - var fullTrace = _supply({ - dimensions: [{ - range: [1, 5], - label: 'A', - values: [1, 4, 3] - }, { - range: [1, 5], - label: 'B', - values: new Float64Array([3, 1.5, 2]), - }, { - range: [1, 5], - label: 'C', - values: new Int32Array([2, 4, 1]), - }], - line: { - color: new Int32Array([0, 1, 2]) - } - }); - expect(Array.isArray(fullTrace.line.color) === true).toEqual(true); - expect(Array.isArray(fullTrace.dimensions[1].values) === true).toEqual(true); - expect(Array.isArray(fullTrace.dimensions[2].values) === true).toEqual(true); - }); - it('\'domain\' specification should have a default', function() { var fullTrace = _supply({}); expect(fullTrace.domain).toEqual({x: [0, 1], y: [0, 1]}); @@ -350,6 +326,30 @@ describe('parcoords initialization tests', function() { color: '#444' }); }); + + it('\'dimensions.values\' and \'line.color\' should convert typed arrays to normal arrays', function() { + var fullTrace = _calc(Lib.extendDeep({}, base, { + dimensions: [{ + range: [1, 5], + label: 'A', + values: [1, 4, 3] + }, { + range: [1, 5], + label: 'B', + values: new Float64Array([3, 1.5, 2]), + }, { + range: [1, 5], + label: 'C', + values: new Int32Array([2, 4, 1]), + }], + line: { + color: new Int32Array([0, 1, 2]) + } + })); + expect(Array.isArray(fullTrace.line.color) === true).toEqual(true); + expect(Array.isArray(fullTrace.dimensions[1].values) === true).toEqual(true); + expect(Array.isArray(fullTrace.dimensions[2].values) === true).toEqual(true); + }); }); }); From e577e49580a35771793f1d0988dbb05b0dc58a5e Mon Sep 17 00:00:00 2001 From: archmoj Date: Mon, 4 Mar 2019 11:44:51 -0500 Subject: [PATCH 5/5] removed isTypedArray function and reused the Lib version --- src/traces/parcoords/calc.js | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/traces/parcoords/calc.js b/src/traces/parcoords/calc.js index 7411de84a9b..4f77081877a 100644 --- a/src/traces/parcoords/calc.js +++ b/src/traces/parcoords/calc.js @@ -46,10 +46,6 @@ function constHalf(len) { return out; } -function isTypedArray(a) { - return !Array.isArray(a) && Lib.isArrayOrTypedArray(a); -} - function convertTypedArray(a) { - return (isTypedArray(a)) ? Array.prototype.slice.call(a) : a; + return (Lib.isTypedArray(a)) ? Array.prototype.slice.call(a) : a; }