From 0003e221568c99556a9452d9d6299d3857aeca34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Mon, 19 Dec 2016 12:28:01 -0500 Subject: [PATCH 1/4] split doCalcdata operation into different loops: - one loop for relinking calcdata - one loop for transform ops - one loop for regular _module.calc in an effort to make transform less intrusive. --- src/plots/plots.js | 58 +++++++++++++++++++++++----------------------- 1 file changed, 29 insertions(+), 29 deletions(-) diff --git a/src/plots/plots.js b/src/plots/plots.js index ad838cd39b2..d293b193bc5 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -1921,8 +1921,9 @@ plots.transition = function(gd, data, layout, traces, frameOpts, transitionOpts) plots.doCalcdata = function(gd, traces) { var axList = Plotly.Axes.list(gd), fullData = gd._fullData, - fullLayout = gd._fullLayout, - i, j; + fullLayout = gd._fullLayout; + + var trace, _module, i, j; // XXX: Is this correct? Needs a closer look so that *some* traces can be recomputed without // *all* needing doCalcdata: @@ -1951,46 +1952,45 @@ plots.doCalcdata = function(gd, traces) { axList[i]._categories = axList[i]._initialCategories.slice(); } + // If traces were specified and this trace was not included, + // then transfer it over from the old calcdata: for(i = 0; i < fullData.length; i++) { - // If traces were specified and this trace was not included, then transfer it over from - // the old calcdata: - if(Array.isArray(traces) && traces.indexOf(i) === -1) { - calcdata[i] = oldCalcdata[i]; - continue; - } - - var trace = fullData[i], - cd = []; - - // If traces were specified and this trace was not included, then transfer it over from - // the old calcdata: if(Array.isArray(traces) && traces.indexOf(i) === -1) { calcdata[i] = oldCalcdata[i]; continue; } + } - var _module; - if(trace.visible === true) { + // transform loop + for(i = 0; i < fullData.length; i++) { + trace = fullData[i]; - // call calcTransform method if any - if(trace.transforms) { + if(trace.visible === true && trace.transforms) { + _module = trace._module; - // we need one round of trace module calc before - // the calc transform to 'fill in' the categories list - // used for example in the data-to-coordinate method - _module = trace._module; - if(_module && _module.calc) _module.calc(gd, trace); + // we need one round of trace module calc before + // the calc transform to 'fill in' the categories list + // used for example in the data-to-coordinate method + if(_module && _module.calc) _module.calc(gd, trace); - for(j = 0; j < trace.transforms.length; j++) { - var transform = trace.transforms[j]; + for(j = 0; j < trace.transforms.length; j++) { + var transform = trace.transforms[j]; - _module = transformsRegistry[transform.type]; - if(_module && _module.calcTransform) { - _module.calcTransform(gd, trace, transform); - } + _module = transformsRegistry[transform.type]; + if(_module && _module.calcTransform) { + _module.calcTransform(gd, trace, transform); } } + } + } + // 'regular' loop + for(i = 0; i < fullData.length; i++) { + var cd = []; + + trace = fullData[i]; + + if(trace.visible === true) { _module = trace._module; if(_module && _module.calc) cd = _module.calc(gd, trace); } From 30a79607811cac8322a25c7dc700ac24661f2d75 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Mon, 19 Dec 2016 12:29:01 -0500 Subject: [PATCH 2/4] clear axis _min and _max after transform calc loop - so that axis autorange is computed after transfroms are made. --- src/plots/plots.js | 6 ++++++ test/jasmine/tests/transform_filter_test.js | 11 +++++++++++ 2 files changed, 17 insertions(+) diff --git a/src/plots/plots.js b/src/plots/plots.js index d293b193bc5..30c55034d5c 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -1984,6 +1984,12 @@ plots.doCalcdata = function(gd, traces) { } } + // clear stuff ... + for(i = 0; i < axList.length; i++) { + axList[i]._min = []; + axList[i]._max = []; + } + // 'regular' loop for(i = 0; i < fullData.length; i++) { var cd = []; diff --git a/test/jasmine/tests/transform_filter_test.js b/test/jasmine/tests/transform_filter_test.js index 59507843486..883e7622ecf 100644 --- a/test/jasmine/tests/transform_filter_test.js +++ b/test/jasmine/tests/transform_filter_test.js @@ -8,6 +8,7 @@ var createGraphDiv = require('../assets/create_graph_div'); var destroyGraphDiv = require('../assets/destroy_graph_div'); var assertDims = require('../assets/assert_dims'); var assertStyle = require('../assets/assert_style'); +var customMatchers = require('../assets/custom_matchers'); describe('filter transforms defaults:', function() { @@ -844,6 +845,10 @@ describe('filter transforms calc:', function() { describe('filter transforms interactions', function() { 'use strict'; + beforeAll(function() { + jasmine.addMatchers(customMatchers); + }); + var mockData0 = [{ x: [-2, -1, -2, 0, 1, 2, 3], y: [1, 2, 3, 1, 2, 3, 1], @@ -898,6 +903,9 @@ describe('filter transforms interactions', function() { assertUid(gd); assertStyle(dims, ['rgb(255, 0, 0)'], [1]); + expect(gd._fullLayout.xaxis.range).toBeCloseToArray([0.87, 3.13]); + expect(gd._fullLayout.yaxis.range).toBeCloseToArray([0.85, 3.15]); + return Plotly.restyle(gd, 'marker.color', 'blue'); }).then(function() { expect(gd._fullData[0].marker.color).toEqual('blue'); @@ -915,6 +923,9 @@ describe('filter transforms interactions', function() { assertUid(gd); assertStyle([1], ['rgb(255, 0, 0)'], [1]); + expect(gd._fullLayout.xaxis.range).toBeCloseToArray([2, 4]); + expect(gd._fullLayout.yaxis.range).toBeCloseToArray([0, 2]); + done(); }); }); From 775dbb5ccdef7884c0e6d95c6b5721b947e8d1a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 4 Jan 2017 14:28:04 -0500 Subject: [PATCH 3/4] transforms: clear _categories after transform calc loop - so that only post-transform categories are displayed on graph --- src/plots/plots.js | 3 +- test/jasmine/tests/transform_filter_test.js | 44 +++++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/src/plots/plots.js b/src/plots/plots.js index 30c55034d5c..5a3ec4d2881 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -1984,10 +1984,11 @@ plots.doCalcdata = function(gd, traces) { } } - // clear stuff ... + // clear stuff that should recomputed in 'regular' loop for(i = 0; i < axList.length; i++) { axList[i]._min = []; axList[i]._max = []; + axList[i]._categories = []; } // 'regular' loop diff --git a/test/jasmine/tests/transform_filter_test.js b/test/jasmine/tests/transform_filter_test.js index 883e7622ecf..9eb8e1e6032 100644 --- a/test/jasmine/tests/transform_filter_test.js +++ b/test/jasmine/tests/transform_filter_test.js @@ -999,4 +999,48 @@ describe('filter transforms interactions', function() { done(); }); }); + + it('should update axie categories', function(done) { + var data = [{ + type: 'bar', + x: ['a', 'b', 'c', 'd', 'e', 'f', 'g'], + y: [1, 10, 100, 25, 50, -25, 100], + transforms: [{ + type: 'filter', + operation: '<', + value: 10, + target: [1, 10, 100, 25, 50, -25, 100] + }] + }]; + + var gd = createGraphDiv(); + + Plotly.plot(gd, data).then(function() { + expect(gd._fullLayout.xaxis._categories).toEqual(['a', 'f']); + expect(gd._fullLayout.yaxis._categories).toEqual([]); + + return Plotly.addTraces(gd, [{ + type: 'bar', + x: ['h', 'i'], + y: [2, 1], + transforms: [{ + type: 'filter', + operation: '=', + value: 'i' + }] + }]); + }) + .then(function() { + expect(gd._fullLayout.xaxis._categories).toEqual(['a', 'f', 'i']); + expect(gd._fullLayout.yaxis._categories).toEqual([]); + + return Plotly.deleteTraces(gd, [0]); + }) + .then(function() { + expect(gd._fullLayout.xaxis._categories).toEqual(['i']); + expect(gd._fullLayout.yaxis._categories).toEqual([]); + + }) + .then(done); + }); }); From c9044fa162cecf8c329d10ad5003d3a8644e9b26 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 4 Jan 2017 15:11:35 -0500 Subject: [PATCH 4/4] only clear ax field before regular loop when transforms are present --- src/plots/plots.js | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/plots/plots.js b/src/plots/plots.js index 98757df2a54..a1d853d40ef 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -1961,6 +1961,8 @@ plots.doCalcdata = function(gd, traces) { } } + var hasCalcTransform = false; + // transform loop for(i = 0; i < fullData.length; i++) { trace = fullData[i]; @@ -1978,6 +1980,7 @@ plots.doCalcdata = function(gd, traces) { _module = transformsRegistry[transform.type]; if(_module && _module.calcTransform) { + hasCalcTransform = true; _module.calcTransform(gd, trace, transform); } } @@ -1985,10 +1988,12 @@ plots.doCalcdata = function(gd, traces) { } // clear stuff that should recomputed in 'regular' loop - for(i = 0; i < axList.length; i++) { - axList[i]._min = []; - axList[i]._max = []; - axList[i]._categories = []; + if(hasCalcTransform) { + for(i = 0; i < axList.length; i++) { + axList[i]._min = []; + axList[i]._max = []; + axList[i]._categories = []; + } } // 'regular' loop