From 1ddbd93deb65eb07ab166885b9682efcdd86fa4a Mon Sep 17 00:00:00 2001 From: Robert Monfera Date: Wed, 25 Oct 2017 23:52:33 +0200 Subject: [PATCH 1/8] inverse mapping --- src/transforms/aggregate.js | 12 +++++++++-- .../jasmine/tests/transform_aggregate_test.js | 20 ++++++++++++++++--- 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/src/transforms/aggregate.js b/src/transforms/aggregate.js index 1f060b3b106..1da344de89d 100644 --- a/src/transforms/aggregate.js +++ b/src/transforms/aggregate.js @@ -215,20 +215,28 @@ exports.calcTransform = function(gd, trace, opts) { var groupArray = Lib.getTargetArray(trace, {target: groups}); if(!groupArray) return; - var i, vi, groupIndex; + var i, vi, groupIndex, newGrouping; var groupIndices = {}; + var groupToPoints = {}; + var indexToPoints = {}; var groupings = []; for(i = 0; i < groupArray.length; i++) { vi = groupArray[i]; groupIndex = groupIndices[vi]; if(groupIndex === undefined) { groupIndices[vi] = groupings.length; - groupings.push([i]); + newGrouping = [i]; + groupings.push(newGrouping); + groupToPoints[vi] = newGrouping; + indexToPoints[groupIndices[vi]] = newGrouping; } else groupings[groupIndex].push(i); } + opts.groupToPoints = groupToPoints; + opts.indexToPoints = indexToPoints; + var aggregations = opts.aggregations; for(i = 0; i < aggregations.length; i++) { diff --git a/test/jasmine/tests/transform_aggregate_test.js b/test/jasmine/tests/transform_aggregate_test.js index 8e9cc9db8ea..e5398a0a8b8 100644 --- a/test/jasmine/tests/transform_aggregate_test.js +++ b/test/jasmine/tests/transform_aggregate_test.js @@ -59,6 +59,10 @@ describe('aggregate', function() { expect(traceOut.marker.opacity).toEqual([0.6, 'boo']); expect(traceOut.marker.line.color).toEqual(['the end', 3.3]); expect(traceOut.marker.line.width).toEqual([4, 1]); + + var transform = traceOut.transforms[0]; + var inverseMapping = transform.indexToPoints; + expect(inverseMapping).toEqual({0: [0, 2, 3, 4], 1: [1]}); }); it('handles all funcs except sum for date data', function() { @@ -163,6 +167,10 @@ describe('aggregate', function() { expect(traceOut.y).toEqual(['b', undefined]); // category average: can result in fractional categories -> rounds (0.5 rounds to 1) expect(traceOut.text).toEqual(['b', 'b']); + + var transform = traceOut.transforms[0]; + var inverseMapping = transform.indexToPoints; + expect(inverseMapping).toEqual({0: [0, 1], 1: [2, 3]}); }); it('can aggregate on an existing data array', function() { @@ -185,10 +193,12 @@ describe('aggregate', function() { expect(traceOut.x).toEqual([8, 7]); expect(traceOut.y).toBeCloseToArray([16 / 3, 7], 5); expect(traceOut.marker.size).toEqual([10, 20]); + + var transform = traceOut.transforms[0]; + var inverseMapping = transform.indexToPoints; + expect(inverseMapping).toEqual({0: [0, 1, 4], 1: [2, 3]}); }); - // Regression test - throws before fix: - // https://github.com/plotly/plotly.js/issues/2024 it('can handle case where aggregation array is missing', function() { Plotly.newPlot(gd, [{ x: [1, 2, 3, 4, 5], @@ -205,6 +215,10 @@ describe('aggregate', function() { expect(traceOut.x).toEqual([1, 3]); expect(traceOut.y).toEqual([2, 6]); expect(traceOut.marker.size).toEqual([10, 20]); + + var transform = traceOut.transforms[0]; + var inverseMapping = transform.indexToPoints; + expect(inverseMapping).toEqual({0: [0, 1, 4], 1: [2, 3]}); }); it('handles median, mode, rms, & stddev for numeric data', function() { @@ -257,7 +271,7 @@ describe('aggregate', function() { aggregations: [ {target: 'x', func: 'sum'}, {target: 'x', func: 'avg'}, - {target: 'y', func: 'avg'}, + {target: 'y', func: 'avg'} ] }] }]); From e1589bce70da7bd763ded704c7f2b4422029b5ee Mon Sep 17 00:00:00 2001 From: Robert Monfera Date: Thu, 26 Oct 2017 21:27:23 +0200 Subject: [PATCH 2/8] removing unused attributes --- src/transforms/aggregate.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/transforms/aggregate.js b/src/transforms/aggregate.js index 1da344de89d..73c80f5ce84 100644 --- a/src/transforms/aggregate.js +++ b/src/transforms/aggregate.js @@ -218,7 +218,6 @@ exports.calcTransform = function(gd, trace, opts) { var i, vi, groupIndex, newGrouping; var groupIndices = {}; - var groupToPoints = {}; var indexToPoints = {}; var groupings = []; for(i = 0; i < groupArray.length; i++) { @@ -228,13 +227,11 @@ exports.calcTransform = function(gd, trace, opts) { groupIndices[vi] = groupings.length; newGrouping = [i]; groupings.push(newGrouping); - groupToPoints[vi] = newGrouping; indexToPoints[groupIndices[vi]] = newGrouping; } else groupings[groupIndex].push(i); } - opts.groupToPoints = groupToPoints; opts.indexToPoints = indexToPoints; var aggregations = opts.aggregations; From caf707e90c4bdf6e0688f9db6ef75305999ee9b8 Mon Sep 17 00:00:00 2001 From: Robert Monfera Date: Thu, 26 Oct 2017 21:29:28 +0200 Subject: [PATCH 3/8] working, cascading filter transform points --- src/transforms/filter.js | 31 +++++++++++++++++++-- test/jasmine/tests/transform_filter_test.js | 10 +++++-- 2 files changed, 36 insertions(+), 5 deletions(-) diff --git a/src/transforms/filter.js b/src/transforms/filter.js index 6e948651558..8cdf85633c7 100644 --- a/src/transforms/filter.js +++ b/src/transforms/filter.js @@ -170,6 +170,8 @@ exports.calcTransform = function(gd, trace, opts) { var d2c = Axes.getDataToCoordFunc(gd, trace, target, targetArray); var filterFunc = getFilterFunc(opts, d2c, targetCalendar); var originalArrays = {}; + var indexToPoints = {}; + var index = 0; function forAllAttrs(fn, index) { for(var j = 0; j < arrayAttrs.length; j++) { @@ -204,10 +206,33 @@ exports.calcTransform = function(gd, trace, opts) { forAllAttrs(initFn); // loop through filter array, fill trace arrays if passed - for(var i = 0; i < len; i++) { - var passed = filterFunc(targetArray[i]); - if(passed) forAllAttrs(fillFn, i); + + var prevTransforms = trace.transforms.filter(function(tr) {return tr.indexToPoints;}); + + if(prevTransforms.length > 0) { + + var prevTransform = prevTransforms[prevTransforms.length - 1]; + var prevIndexToPoints = prevTransform.indexToPoints; + + for(var i = 0; i < len; i++) { + var passed = filterFunc(targetArray[i]); + if(passed) { + forAllAttrs(fillFn, i); + indexToPoints[index++] = prevIndexToPoints[i]; + } + } + } else { + for(var i = 0; i < len; i++) { + var passed = filterFunc(targetArray[i]); + if(passed) { + forAllAttrs(fillFn, i); + indexToPoints[index++] = [i]; + } + } } + + opts.indexToPoints = indexToPoints; + console.log(indexToPoints) }; function getFilterFunc(opts, d2c, targetCalendar) { diff --git a/test/jasmine/tests/transform_filter_test.js b/test/jasmine/tests/transform_filter_test.js index d338251a546..3e043af4bb3 100644 --- a/test/jasmine/tests/transform_filter_test.js +++ b/test/jasmine/tests/transform_filter_test.js @@ -62,7 +62,7 @@ describe('filter transforms defaults:', function() { traceIn = { x: [1, 2, 3], transforms: [{ - type: 'filter', + type: 'filter' }, { type: 'filter', target: 0 @@ -143,6 +143,7 @@ describe('filter transforms calc:', function() { expect(out[0].x).toEqual([0, 1]); expect(out[0].y).toEqual([1, 2]); expect(out[0].z).toEqual(['2016-10-21', '2016-12-02']); + expect(out[0].transforms[0].indexToPoints).toEqual({0: [3], 1 :[4]}); }); it('should use the calendar from the target attribute if target is a string', function() { @@ -261,13 +262,14 @@ describe('filter transforms calc:', function() { expect(out[0].x).toEqual([-2, 2, 3]); expect(out[0].y).toEqual([3, 3, 1]); expect(out[0].marker.color).toEqual([0.3, 0.3, 0.4]); + expect(out[0].transforms[0].indexToPoints).toEqual({0: [2], 1 :[5], 2: [6]}); }); it('filters should handle array on base trace attributes', function() { var out = _transform([Lib.extendDeep({}, base, { hoverinfo: ['x', 'y', 'text', 'name', 'none', 'skip', 'all'], hoverlabel: { - bgcolor: ['red', 'green', 'blue', 'black', 'yellow', 'cyan', 'pink'], + bgcolor: ['red', 'green', 'blue', 'black', 'yellow', 'cyan', 'pink'] }, transforms: [{ type: 'filter', @@ -314,6 +316,8 @@ describe('filter transforms calc:', function() { expect(out[0].x).toEqual([1, 2]); expect(out[0].y).toEqual([2, 3]); + expect(out[0].transforms[0].indexToPoints).toEqual({0: [4], 1 :[5], 2: [6]}); + expect(out[0].transforms[1].indexToPoints).toEqual({0: [4], 1 :[5]}); }); it('filters should chain as AND (case 2)', function() { @@ -339,6 +343,8 @@ describe('filter transforms calc:', function() { expect(out[0].x).toEqual([3]); expect(out[0].y).toEqual([1]); + expect(out[0].transforms[0].indexToPoints).toEqual({0: [4], 1 :[5], 2: [6]}); + expect(out[0].transforms[2].indexToPoints).toEqual({0: [6]}); }); it('should preserve gaps in data when `preservegaps` is turned on', function() { From 979ddea9dd414b0e8f6ad36968b25f04f39c5a20 Mon Sep 17 00:00:00 2001 From: Robert Monfera Date: Thu, 26 Oct 2017 21:35:35 +0200 Subject: [PATCH 4/8] filter points cascading simplification --- src/transforms/filter.js | 31 ++++++--------------- test/jasmine/tests/transform_filter_test.js | 10 +++---- 2 files changed, 14 insertions(+), 27 deletions(-) diff --git a/src/transforms/filter.js b/src/transforms/filter.js index 8cdf85633c7..fe56498ac34 100644 --- a/src/transforms/filter.js +++ b/src/transforms/filter.js @@ -205,34 +205,21 @@ exports.calcTransform = function(gd, trace, opts) { // copy all original array attribute values, and clear arrays in trace forAllAttrs(initFn); - // loop through filter array, fill trace arrays if passed - var prevTransforms = trace.transforms.filter(function(tr) {return tr.indexToPoints;}); + var originalPointsAccessor = prevTransforms.length ? + function(i) {return prevTransforms[prevTransforms.length - 1].indexToPoints[i];} : + function(i) {return [i];}; - if(prevTransforms.length > 0) { - - var prevTransform = prevTransforms[prevTransforms.length - 1]; - var prevIndexToPoints = prevTransform.indexToPoints; - - for(var i = 0; i < len; i++) { - var passed = filterFunc(targetArray[i]); - if(passed) { - forAllAttrs(fillFn, i); - indexToPoints[index++] = prevIndexToPoints[i]; - } - } - } else { - for(var i = 0; i < len; i++) { - var passed = filterFunc(targetArray[i]); - if(passed) { - forAllAttrs(fillFn, i); - indexToPoints[index++] = [i]; - } + // loop through filter array, fill trace arrays if passed + for(var i = 0; i < len; i++) { + var passed = filterFunc(targetArray[i]); + if(passed) { + forAllAttrs(fillFn, i); + indexToPoints[index++] = originalPointsAccessor(i); } } opts.indexToPoints = indexToPoints; - console.log(indexToPoints) }; function getFilterFunc(opts, d2c, targetCalendar) { diff --git a/test/jasmine/tests/transform_filter_test.js b/test/jasmine/tests/transform_filter_test.js index 3e043af4bb3..ee89a37bd42 100644 --- a/test/jasmine/tests/transform_filter_test.js +++ b/test/jasmine/tests/transform_filter_test.js @@ -143,7 +143,7 @@ describe('filter transforms calc:', function() { expect(out[0].x).toEqual([0, 1]); expect(out[0].y).toEqual([1, 2]); expect(out[0].z).toEqual(['2016-10-21', '2016-12-02']); - expect(out[0].transforms[0].indexToPoints).toEqual({0: [3], 1 :[4]}); + expect(out[0].transforms[0].indexToPoints).toEqual({0: [3], 1: [4]}); }); it('should use the calendar from the target attribute if target is a string', function() { @@ -262,7 +262,7 @@ describe('filter transforms calc:', function() { expect(out[0].x).toEqual([-2, 2, 3]); expect(out[0].y).toEqual([3, 3, 1]); expect(out[0].marker.color).toEqual([0.3, 0.3, 0.4]); - expect(out[0].transforms[0].indexToPoints).toEqual({0: [2], 1 :[5], 2: [6]}); + expect(out[0].transforms[0].indexToPoints).toEqual({0: [2], 1: [5], 2: [6]}); }); it('filters should handle array on base trace attributes', function() { @@ -316,8 +316,8 @@ describe('filter transforms calc:', function() { expect(out[0].x).toEqual([1, 2]); expect(out[0].y).toEqual([2, 3]); - expect(out[0].transforms[0].indexToPoints).toEqual({0: [4], 1 :[5], 2: [6]}); - expect(out[0].transforms[1].indexToPoints).toEqual({0: [4], 1 :[5]}); + expect(out[0].transforms[0].indexToPoints).toEqual({0: [4], 1: [5], 2: [6]}); + expect(out[0].transforms[1].indexToPoints).toEqual({0: [4], 1: [5]}); }); it('filters should chain as AND (case 2)', function() { @@ -343,7 +343,7 @@ describe('filter transforms calc:', function() { expect(out[0].x).toEqual([3]); expect(out[0].y).toEqual([1]); - expect(out[0].transforms[0].indexToPoints).toEqual({0: [4], 1 :[5], 2: [6]}); + expect(out[0].transforms[0].indexToPoints).toEqual({0: [4], 1: [5], 2: [6]}); expect(out[0].transforms[2].indexToPoints).toEqual({0: [6]}); }); From fb9b049aab0d8641977c187ced8028f3d7126b52 Mon Sep 17 00:00:00 2001 From: Robert Monfera Date: Thu, 26 Oct 2017 22:51:57 +0200 Subject: [PATCH 5/8] test for heterogeneous transforms --- test/jasmine/tests/transform_multi_test.js | 50 +++++++++++++++++++++- 1 file changed, 49 insertions(+), 1 deletion(-) diff --git a/test/jasmine/tests/transform_multi_test.js b/test/jasmine/tests/transform_multi_test.js index a01b219fa09..a845a38cf7e 100644 --- a/test/jasmine/tests/transform_multi_test.js +++ b/test/jasmine/tests/transform_multi_test.js @@ -245,7 +245,7 @@ describe('multiple transforms:', function() { groups: ['a', 'a', 'b', 'a', 'b', 'b', 'a'], styles: [{ target: 'a', - value: {marker: {color: 'red'}}, + value: {marker: {color: 'red'}} }, { target: 'b', value: {marker: {color: 'blue'}} @@ -277,8 +277,56 @@ describe('multiple transforms:', function() { }] }]; + var mockData2 = [{ + x: [1, 2, 3, 4, 5], + y: [2, 3, 1, 7, 9], + marker: {size: [10, 20, 20, 20, 10]}, + transforms: [ + { + type: 'filter', + operation: '>', + value: 2, + target: 'y' + }, + { + type: 'aggregate', + groups: 'marker.size', + aggregations: [ + {target: 'x', func: 'sum'}, //20: 6, 10: 5 + {target: 'y', func: 'avg'} //20: 5, 10: 9 + ] + }, + { + type: 'filter', + operation: '<', + value: 6, + target: 'x' + } + ] + }]; + afterEach(destroyGraphDiv); + fit('Plotly.plot should plot the transform traces - filter|aggregate|filter', function(done) { + var data = Lib.extendDeep([], mockData2); + + Plotly.plot(gd, data).then(function() { + expect(gd.data.length).toEqual(1); + + // this would be the result if we didn't have a second filter + // expect(gd._fullData[0].x).toEqual([6, 5]); + // expect(gd._fullData[0].y).toEqual([5, 9]); + // expect(gd._fullData[0].marker.size).toEqual([20, 10]); + + expect(gd._fullData[0].x).toEqual([5]); + expect(gd._fullData[0].y).toEqual([9]); + expect(gd._fullData[0].marker.size).toEqual([10]); + + done(); + }); + }); + + it('Plotly.plot should plot the transform traces', function(done) { var data = Lib.extendDeep([], mockData0); From 039d9b63b6568e44f9699af65dc679ee28ac99ea Mon Sep 17 00:00:00 2001 From: Robert Monfera Date: Thu, 26 Oct 2017 23:23:04 +0200 Subject: [PATCH 6/8] aggregation to also use points from the preceding `transform` step --- src/transforms/aggregate.js | 13 +++++++++++-- test/jasmine/tests/transform_multi_test.js | 16 ++++++++++------ 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/src/transforms/aggregate.js b/src/transforms/aggregate.js index 73c80f5ce84..bcc376b1067 100644 --- a/src/transforms/aggregate.js +++ b/src/transforms/aggregate.js @@ -220,6 +220,12 @@ exports.calcTransform = function(gd, trace, opts) { var groupIndices = {}; var indexToPoints = {}; var groupings = []; + + var prevTransforms = trace.transforms.filter(function(tr) {return tr.indexToPoints;}); + var originalPointsAccessor = prevTransforms.length ? + function(i) {return prevTransforms[prevTransforms.length - 1].indexToPoints[i];} : + function(i) {return [i];}; + for(i = 0; i < groupArray.length; i++) { vi = groupArray[i]; groupIndex = groupIndices[vi]; @@ -227,9 +233,12 @@ exports.calcTransform = function(gd, trace, opts) { groupIndices[vi] = groupings.length; newGrouping = [i]; groupings.push(newGrouping); - indexToPoints[groupIndices[vi]] = newGrouping; + indexToPoints[groupIndices[vi]] = originalPointsAccessor(i); + } + else { + groupings[groupIndex].push(i); + indexToPoints[groupIndices[vi]] = indexToPoints[groupIndices[vi]].concat(originalPointsAccessor(i)); } - else groupings[groupIndex].push(i); } opts.indexToPoints = indexToPoints; diff --git a/test/jasmine/tests/transform_multi_test.js b/test/jasmine/tests/transform_multi_test.js index a845a38cf7e..f66e44406b4 100644 --- a/test/jasmine/tests/transform_multi_test.js +++ b/test/jasmine/tests/transform_multi_test.js @@ -278,8 +278,8 @@ describe('multiple transforms:', function() { }]; var mockData2 = [{ - x: [1, 2, 3, 4, 5], - y: [2, 3, 1, 7, 9], + x: [1, 2, 3, 4, 5], + y: [2, 3, 1, 7, 9], marker: {size: [10, 20, 20, 20, 10]}, transforms: [ { @@ -292,8 +292,8 @@ describe('multiple transforms:', function() { type: 'aggregate', groups: 'marker.size', aggregations: [ - {target: 'x', func: 'sum'}, //20: 6, 10: 5 - {target: 'y', func: 'avg'} //20: 5, 10: 9 + {target: 'x', func: 'sum'}, // 20: 6, 10: 5 + {target: 'y', func: 'avg'} // 20: 5, 10: 9 ] }, { @@ -307,13 +307,13 @@ describe('multiple transforms:', function() { afterEach(destroyGraphDiv); - fit('Plotly.plot should plot the transform traces - filter|aggregate|filter', function(done) { + it('Plotly.plot should plot the transform traces - filter|aggregate|filter', function(done) { var data = Lib.extendDeep([], mockData2); Plotly.plot(gd, data).then(function() { expect(gd.data.length).toEqual(1); - // this would be the result if we didn't have a second filter + // this would be the result if we didn't have a second filter - kept for test case overview // expect(gd._fullData[0].x).toEqual([6, 5]); // expect(gd._fullData[0].y).toEqual([5, 9]); // expect(gd._fullData[0].marker.size).toEqual([20, 10]); @@ -322,6 +322,10 @@ describe('multiple transforms:', function() { expect(gd._fullData[0].y).toEqual([9]); expect(gd._fullData[0].marker.size).toEqual([10]); + expect(gd._fullData[0].transforms[0].indexToPoints).toEqual({0: [1], 1: [3], 2: [4]}); + expect(gd._fullData[0].transforms[1].indexToPoints).toEqual({0: [1, 3], 1: [4]}); + expect(gd._fullData[0].transforms[2].indexToPoints).toEqual({0: [4]}); + done(); }); }); From 70a0f68fa435cc3b560bcc3e95935fdf8da176b9 Mon Sep 17 00:00:00 2001 From: Robert Monfera Date: Fri, 27 Oct 2017 09:13:54 +0200 Subject: [PATCH 7/8] rename to _indexToPoints --- src/transforms/aggregate.js | 6 +++--- src/transforms/filter.js | 6 +++--- test/jasmine/tests/transform_aggregate_test.js | 8 ++++---- test/jasmine/tests/transform_filter_test.js | 12 ++++++------ test/jasmine/tests/transform_multi_test.js | 6 +++--- 5 files changed, 19 insertions(+), 19 deletions(-) diff --git a/src/transforms/aggregate.js b/src/transforms/aggregate.js index bcc376b1067..573150d0473 100644 --- a/src/transforms/aggregate.js +++ b/src/transforms/aggregate.js @@ -221,9 +221,9 @@ exports.calcTransform = function(gd, trace, opts) { var indexToPoints = {}; var groupings = []; - var prevTransforms = trace.transforms.filter(function(tr) {return tr.indexToPoints;}); + var prevTransforms = trace.transforms.filter(function(tr) {return tr._indexToPoints;}); var originalPointsAccessor = prevTransforms.length ? - function(i) {return prevTransforms[prevTransforms.length - 1].indexToPoints[i];} : + function(i) {return prevTransforms[prevTransforms.length - 1]._indexToPoints[i];} : function(i) {return [i];}; for(i = 0; i < groupArray.length; i++) { @@ -241,7 +241,7 @@ exports.calcTransform = function(gd, trace, opts) { } } - opts.indexToPoints = indexToPoints; + opts._indexToPoints = indexToPoints; var aggregations = opts.aggregations; diff --git a/src/transforms/filter.js b/src/transforms/filter.js index fe56498ac34..e3cf912bf37 100644 --- a/src/transforms/filter.js +++ b/src/transforms/filter.js @@ -205,9 +205,9 @@ exports.calcTransform = function(gd, trace, opts) { // copy all original array attribute values, and clear arrays in trace forAllAttrs(initFn); - var prevTransforms = trace.transforms.filter(function(tr) {return tr.indexToPoints;}); + var prevTransforms = trace.transforms.filter(function(tr) {return tr._indexToPoints;}); var originalPointsAccessor = prevTransforms.length ? - function(i) {return prevTransforms[prevTransforms.length - 1].indexToPoints[i];} : + function(i) {return prevTransforms[prevTransforms.length - 1]._indexToPoints[i];} : function(i) {return [i];}; // loop through filter array, fill trace arrays if passed @@ -219,7 +219,7 @@ exports.calcTransform = function(gd, trace, opts) { } } - opts.indexToPoints = indexToPoints; + opts._indexToPoints = indexToPoints; }; function getFilterFunc(opts, d2c, targetCalendar) { diff --git a/test/jasmine/tests/transform_aggregate_test.js b/test/jasmine/tests/transform_aggregate_test.js index e5398a0a8b8..0f7932b2c48 100644 --- a/test/jasmine/tests/transform_aggregate_test.js +++ b/test/jasmine/tests/transform_aggregate_test.js @@ -61,7 +61,7 @@ describe('aggregate', function() { expect(traceOut.marker.line.width).toEqual([4, 1]); var transform = traceOut.transforms[0]; - var inverseMapping = transform.indexToPoints; + var inverseMapping = transform._indexToPoints; expect(inverseMapping).toEqual({0: [0, 2, 3, 4], 1: [1]}); }); @@ -169,7 +169,7 @@ describe('aggregate', function() { expect(traceOut.text).toEqual(['b', 'b']); var transform = traceOut.transforms[0]; - var inverseMapping = transform.indexToPoints; + var inverseMapping = transform._indexToPoints; expect(inverseMapping).toEqual({0: [0, 1], 1: [2, 3]}); }); @@ -195,7 +195,7 @@ describe('aggregate', function() { expect(traceOut.marker.size).toEqual([10, 20]); var transform = traceOut.transforms[0]; - var inverseMapping = transform.indexToPoints; + var inverseMapping = transform._indexToPoints; expect(inverseMapping).toEqual({0: [0, 1, 4], 1: [2, 3]}); }); @@ -217,7 +217,7 @@ describe('aggregate', function() { expect(traceOut.marker.size).toEqual([10, 20]); var transform = traceOut.transforms[0]; - var inverseMapping = transform.indexToPoints; + var inverseMapping = transform._indexToPoints; expect(inverseMapping).toEqual({0: [0, 1, 4], 1: [2, 3]}); }); diff --git a/test/jasmine/tests/transform_filter_test.js b/test/jasmine/tests/transform_filter_test.js index ee89a37bd42..7caee22f0e6 100644 --- a/test/jasmine/tests/transform_filter_test.js +++ b/test/jasmine/tests/transform_filter_test.js @@ -143,7 +143,7 @@ describe('filter transforms calc:', function() { expect(out[0].x).toEqual([0, 1]); expect(out[0].y).toEqual([1, 2]); expect(out[0].z).toEqual(['2016-10-21', '2016-12-02']); - expect(out[0].transforms[0].indexToPoints).toEqual({0: [3], 1: [4]}); + expect(out[0].transforms[0]._indexToPoints).toEqual({0: [3], 1: [4]}); }); it('should use the calendar from the target attribute if target is a string', function() { @@ -262,7 +262,7 @@ describe('filter transforms calc:', function() { expect(out[0].x).toEqual([-2, 2, 3]); expect(out[0].y).toEqual([3, 3, 1]); expect(out[0].marker.color).toEqual([0.3, 0.3, 0.4]); - expect(out[0].transforms[0].indexToPoints).toEqual({0: [2], 1: [5], 2: [6]}); + expect(out[0].transforms[0]._indexToPoints).toEqual({0: [2], 1: [5], 2: [6]}); }); it('filters should handle array on base trace attributes', function() { @@ -316,8 +316,8 @@ describe('filter transforms calc:', function() { expect(out[0].x).toEqual([1, 2]); expect(out[0].y).toEqual([2, 3]); - expect(out[0].transforms[0].indexToPoints).toEqual({0: [4], 1: [5], 2: [6]}); - expect(out[0].transforms[1].indexToPoints).toEqual({0: [4], 1: [5]}); + expect(out[0].transforms[0]._indexToPoints).toEqual({0: [4], 1: [5], 2: [6]}); + expect(out[0].transforms[1]._indexToPoints).toEqual({0: [4], 1: [5]}); }); it('filters should chain as AND (case 2)', function() { @@ -343,8 +343,8 @@ describe('filter transforms calc:', function() { expect(out[0].x).toEqual([3]); expect(out[0].y).toEqual([1]); - expect(out[0].transforms[0].indexToPoints).toEqual({0: [4], 1: [5], 2: [6]}); - expect(out[0].transforms[2].indexToPoints).toEqual({0: [6]}); + expect(out[0].transforms[0]._indexToPoints).toEqual({0: [4], 1: [5], 2: [6]}); + expect(out[0].transforms[2]._indexToPoints).toEqual({0: [6]}); }); it('should preserve gaps in data when `preservegaps` is turned on', function() { diff --git a/test/jasmine/tests/transform_multi_test.js b/test/jasmine/tests/transform_multi_test.js index f66e44406b4..94deb0f5505 100644 --- a/test/jasmine/tests/transform_multi_test.js +++ b/test/jasmine/tests/transform_multi_test.js @@ -322,9 +322,9 @@ describe('multiple transforms:', function() { expect(gd._fullData[0].y).toEqual([9]); expect(gd._fullData[0].marker.size).toEqual([10]); - expect(gd._fullData[0].transforms[0].indexToPoints).toEqual({0: [1], 1: [3], 2: [4]}); - expect(gd._fullData[0].transforms[1].indexToPoints).toEqual({0: [1, 3], 1: [4]}); - expect(gd._fullData[0].transforms[2].indexToPoints).toEqual({0: [4]}); + expect(gd._fullData[0].transforms[0]._indexToPoints).toEqual({0: [1], 1: [3], 2: [4]}); + expect(gd._fullData[0].transforms[1]._indexToPoints).toEqual({0: [1, 3], 1: [4]}); + expect(gd._fullData[0].transforms[2]._indexToPoints).toEqual({0: [4]}); done(); }); From ce3a118b70ca4e6aead0710a95cdd43602cf648d Mon Sep 17 00:00:00 2001 From: Robert Monfera Date: Fri, 27 Oct 2017 14:11:59 +0200 Subject: [PATCH 8/8] extracting out common function --- src/transforms/aggregate.js | 8 +++----- src/transforms/filter.js | 6 ++---- src/transforms/helpers.js | 24 ++++++++++++++++++++++++ 3 files changed, 29 insertions(+), 9 deletions(-) create mode 100644 src/transforms/helpers.js diff --git a/src/transforms/aggregate.js b/src/transforms/aggregate.js index 573150d0473..a144d91d11f 100644 --- a/src/transforms/aggregate.js +++ b/src/transforms/aggregate.js @@ -11,6 +11,7 @@ var Axes = require('../plots/cartesian/axes'); var Lib = require('../lib'); var PlotSchema = require('../plot_api/plot_schema'); +var pointsAccessorFunction = require('./helpers').pointsAccessorFunction; var BADNUM = require('../constants/numerical').BADNUM; exports.moduleType = 'transform'; @@ -221,10 +222,7 @@ exports.calcTransform = function(gd, trace, opts) { var indexToPoints = {}; var groupings = []; - var prevTransforms = trace.transforms.filter(function(tr) {return tr._indexToPoints;}); - var originalPointsAccessor = prevTransforms.length ? - function(i) {return prevTransforms[prevTransforms.length - 1]._indexToPoints[i];} : - function(i) {return [i];}; + var originalPointsAccessor = pointsAccessorFunction(trace.transforms, opts); for(i = 0; i < groupArray.length; i++) { vi = groupArray[i]; @@ -237,7 +235,7 @@ exports.calcTransform = function(gd, trace, opts) { } else { groupings[groupIndex].push(i); - indexToPoints[groupIndices[vi]] = indexToPoints[groupIndices[vi]].concat(originalPointsAccessor(i)); + indexToPoints[groupIndices[vi]] = (indexToPoints[groupIndices[vi]] || []).concat(originalPointsAccessor(i)); } } diff --git a/src/transforms/filter.js b/src/transforms/filter.js index e3cf912bf37..1fbea13bc31 100644 --- a/src/transforms/filter.js +++ b/src/transforms/filter.js @@ -11,6 +11,7 @@ var Lib = require('../lib'); var Registry = require('../registry'); var Axes = require('../plots/cartesian/axes'); +var pointsAccessorFunction = require('./helpers').pointsAccessorFunction; var COMPARISON_OPS = ['=', '!=', '<', '>=', '>', '<=']; var INTERVAL_OPS = ['[]', '()', '[)', '(]', '][', ')(', '](', ')[']; @@ -205,10 +206,7 @@ exports.calcTransform = function(gd, trace, opts) { // copy all original array attribute values, and clear arrays in trace forAllAttrs(initFn); - var prevTransforms = trace.transforms.filter(function(tr) {return tr._indexToPoints;}); - var originalPointsAccessor = prevTransforms.length ? - function(i) {return prevTransforms[prevTransforms.length - 1]._indexToPoints[i];} : - function(i) {return [i];}; + var originalPointsAccessor = pointsAccessorFunction(trace.transforms, opts); // loop through filter array, fill trace arrays if passed for(var i = 0; i < len; i++) { diff --git a/src/transforms/helpers.js b/src/transforms/helpers.js new file mode 100644 index 00000000000..4b27d53a430 --- /dev/null +++ b/src/transforms/helpers.js @@ -0,0 +1,24 @@ +/** +* Copyright 2012-2017, Plotly, Inc. +* All rights reserved. +* +* This source code is licensed under the MIT license found in the +* LICENSE file in the root directory of this source tree. +*/ + +'use strict'; + +exports.pointsAccessorFunction = function(transforms, opts) { + var tr; + var prevIndexToPoints; + for(var i = 0; i < transforms.length; i++) { + tr = transforms[i]; + if(tr === opts) break; + if(!tr._indexToPoints || tr.enabled === false) continue; + prevIndexToPoints = tr._indexToPoints; + } + var originalPointsAccessor = prevIndexToPoints ? + function(i) {return prevIndexToPoints[i];} : + function(i) {return [i];}; + return originalPointsAccessor; +};