Skip to content

Commit fcc459d

Browse files
alexcjohnsonalexcjohnson
committed
fix #2508, fix #2470 - problems with Plotly.react and aggregate transforms
also makes transforms work with parcoords (and by extension splom) by having findArrayAttributes dig into container arrays
1 parent ed24765 commit fcc459d

File tree

5 files changed

+243
-26
lines changed

5 files changed

+243
-26
lines changed

‎src/plot_api/plot_api.js

+7-1
Original file line numberDiff line numberDiff line change
@@ -2267,7 +2267,10 @@ exports.react = function(gd, data, layout, config) {
22672267
gd.layout = layout || {};
22682268
helpers.cleanLayout(gd.layout);
22692269

2270-
Plots.supplyDefaults(gd);
2270+
// "true" skips updating calcdata and remapping arrays from calcTransforms,
2271+
// which supplyDefaults usually does at the end, but we may need to NOT do
2272+
// if the diff (which we haven't determined yet) says we'll recalc
2273+
Plots.supplyDefaults(gd, true);
22712274

22722275
var newFullData = gd._fullData;
22732276
var newFullLayout = gd._fullLayout;
@@ -2289,6 +2292,9 @@ exports.react = function(gd, data, layout, config) {
22892292

22902293
// clear calcdata if required
22912294
if(restyleFlags.calc || relayoutFlags.calc) gd.calcdata = undefined;
2295+
// otherwise do the calcdata updates and calcTransform array remaps that we skipped earlier
2296+
else Plots.supplyDefaultsUpdateCalc(gd.calcdata, newFullData);
2297+
22922298
if(relayoutFlags.margins) helpers.clearAxisAutomargins(gd);
22932299

22942300
// Note: what restyle/relayout use impliedEdits and clearAxisTypes for

‎src/plot_api/plot_schema.js

+33-12
Original file line numberDiff line numberDiff line change
@@ -171,9 +171,12 @@ exports.isValObject = function(obj) {
171171
exports.findArrayAttributes = function(trace) {
172172
var arrayAttributes = [];
173173
var stack = [];
174+
var isArrayStack = [];
175+
var baseContainer, baseAttrName;
174176

175177
function callback(attr, attrName, attrs, level) {
176178
stack = stack.slice(0, level).concat([attrName]);
179+
isArrayStack = isArrayStack.slice(0, level).concat([attr && attr._isLinkedToArray]);
177180

178181
var splittableAttr = (
179182
attr &&
@@ -190,33 +193,51 @@ exports.findArrayAttributes = function(trace) {
190193

191194
if(!splittableAttr) return;
192195

193-
var astr = toAttrString(stack);
194-
var val = Lib.nestedProperty(trace, astr).get();
195-
if(!Lib.isArrayOrTypedArray(val)) return;
196-
197-
arrayAttributes.push(astr);
196+
crawlIntoTrace(baseContainer, 0, '');
198197
}
199198

200-
function toAttrString(stack) {
201-
return stack.join('.');
199+
function crawlIntoTrace(container, i, astrPartial) {
200+
var item = container[stack[i]];
201+
var newAstrPartial = astrPartial + stack[i];
202+
if(i === stack.length - 1) {
203+
if(Lib.isArrayOrTypedArray(item)) {
204+
arrayAttributes.push(baseAttrName + newAstrPartial);
205+
}
206+
}
207+
else {
208+
if(isArrayStack[i]) {
209+
if(Array.isArray(item)) {
210+
for(var j = 0; j < item.length; j++) {
211+
if(Lib.isPlainObject(item[j])) {
212+
crawlIntoTrace(item[j], i + 1, newAstrPartial + '[' + j + '].');
213+
}
214+
}
215+
}
216+
}
217+
else if(Lib.isPlainObject(item)) {
218+
crawlIntoTrace(item, i + 1, newAstrPartial + '.');
219+
}
220+
}
202221
}
203222

223+
baseContainer = trace;
224+
baseAttrName = '';
204225
exports.crawl(baseAttributes, callback);
205226
if(trace._module && trace._module.attributes) {
206227
exports.crawl(trace._module.attributes, callback);
207228
}
208229

209-
if(trace.transforms) {
210-
var transforms = trace.transforms;
211-
230+
var transforms = trace.transforms;
231+
if(transforms) {
212232
for(var i = 0; i < transforms.length; i++) {
213233
var transform = transforms[i];
214234
var module = transform._module;
215235

216236
if(module) {
217-
stack = ['transforms[' + i + ']'];
237+
baseAttrName = 'transforms[' + i + '].';
238+
baseContainer = transform;
218239

219-
exports.crawl(module.attributes, callback, 1);
240+
exports.crawl(module.attributes, callback);
220241
}
221242
}
222243
}

‎src/plots/plots.js

+17-13
Original file line numberDiff line numberDiff line change
@@ -274,7 +274,7 @@ var extraFormatKeys = [
274274
// gd._fullLayout._transformModules
275275
// is a list of all the transform modules invoked.
276276
//
277-
plots.supplyDefaults = function(gd) {
277+
plots.supplyDefaults = function(gd, skipCalcUpdate) {
278278
var oldFullLayout = gd._fullLayout || {};
279279

280280
if(oldFullLayout._skipDefaults) {
@@ -458,24 +458,28 @@ plots.supplyDefaults = function(gd) {
458458
}
459459

460460
// update object references in calcdata
461-
if(oldCalcdata.length === newFullData.length) {
462-
for(i = 0; i < newFullData.length; i++) {
463-
var newTrace = newFullData[i];
464-
var cd0 = oldCalcdata[i][0];
465-
if(cd0 && cd0.trace) {
466-
if(cd0.trace._hasCalcTransform) {
467-
remapTransformedArrays(cd0, newTrace);
468-
} else {
469-
cd0.trace = newTrace;
470-
}
471-
}
472-
}
461+
if(!skipCalcUpdate && oldCalcdata.length === newFullData.length) {
462+
plots.supplyDefaultsUpdateCalc(oldCalcdata, newFullData);
473463
}
474464

475465
// sort base plot modules for consistent ordering
476466
newFullLayout._basePlotModules.sort(sortBasePlotModules);
477467
};
478468

469+
plots.supplyDefaultsUpdateCalc = function(oldCalcdata, newFullData) {
470+
for(var i = 0; i < newFullData.length; i++) {
471+
var newTrace = newFullData[i];
472+
var cd0 = oldCalcdata[i][0];
473+
if(cd0 && cd0.trace) {
474+
if(cd0.trace._hasCalcTransform) {
475+
remapTransformedArrays(cd0, newTrace);
476+
} else {
477+
cd0.trace = newTrace;
478+
}
479+
}
480+
}
481+
};
482+
479483
/**
480484
* Make a container for collecting subplots we need to display.
481485
*

‎src/transforms/aggregate.js

+2
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,8 @@ exports.calcTransform = function(gd, trace, opts) {
254254
enabled: true
255255
});
256256
}
257+
258+
trace._length = groupings.length;
257259
};
258260

259261
function aggregateOneArray(gd, trace, groupings, aggregation) {

‎test/jasmine/tests/plot_api_test.js

+184
Original file line numberDiff line numberDiff line change
@@ -2955,6 +2955,190 @@ describe('Test plot api', function() {
29552955
.then(done);
29562956
});
29572957

2958+
function aggregatedPie(i) {
2959+
var labels = i <= 1 ?
2960+
['A', 'B', 'A', 'C', 'A', 'B', 'C', 'A', 'B', 'C', 'A'] :
2961+
['X', 'Y', 'Z', 'Z', 'Y', 'Z', 'X', 'Z', 'Y', 'Z', 'X'];
2962+
var trace = {
2963+
type: 'pie',
2964+
values: [4, 1, 4, 4, 1, 4, 4, 2, 1, 1, 15],
2965+
labels: labels,
2966+
transforms: [{
2967+
type: 'aggregate',
2968+
groups: labels,
2969+
aggregations: [{target: 'values', func: 'sum'}]
2970+
}]
2971+
};
2972+
return {
2973+
data: [trace],
2974+
layout: {
2975+
datarevision: i,
2976+
colorway: ['red', 'orange', 'yellow', 'green', 'blue', 'violet']
2977+
}
2978+
};
2979+
}
2980+
2981+
var aggPie1CD = [[
2982+
{v: 26, label: 'A', color: 'red', i: 0},
2983+
{v: 9, label: 'C', color: 'orange', i: 2},
2984+
{v: 6, label: 'B', color: 'yellow', i: 1}
2985+
]];
2986+
2987+
var aggPie2CD = [[
2988+
{v: 23, label: 'X', color: 'red', i: 0},
2989+
{v: 15, label: 'Z', color: 'orange', i: 2},
2990+
{v: 3, label: 'Y', color: 'yellow', i: 1}
2991+
]];
2992+
2993+
function aggregatedScatter(i) {
2994+
return {
2995+
data: [{
2996+
x: [1, 2, 3, 4, 6, 5],
2997+
y: [2, 1, 3, 5, 6, 4],
2998+
transforms: [{
2999+
type: 'aggregate',
3000+
groups: [1, -1, 1, -1, 1, -1],
3001+
aggregations: i > 1 ? [{func: 'last', target: 'x'}] : []
3002+
}]
3003+
}],
3004+
layout: {daterevision: i + 10}
3005+
};
3006+
}
3007+
3008+
var aggScatter1CD = [[
3009+
{x: 1, y: 2, i: 0},
3010+
{x: 2, y: 1, i: 1}
3011+
]];
3012+
3013+
var aggScatter2CD = [[
3014+
{x: 6, y: 2, i: 0},
3015+
{x: 5, y: 1, i: 1}
3016+
]];
3017+
3018+
function aggregatedParcoords(i) {
3019+
return {
3020+
data: [{
3021+
type: 'parcoords',
3022+
dimensions: [
3023+
{label: 'A', values: [1, 2, 3, 4]},
3024+
{label: 'B', values: [4, 3, 2, 1]}
3025+
],
3026+
transforms: i ? [{
3027+
type: 'aggregate',
3028+
groups: [1, 2, 1, 2],
3029+
aggregations: [
3030+
{target: 'dimensions[0].values', func: i > 1 ? 'avg' : 'first'},
3031+
{target: 'dimensions[1].values', func: i > 1 ? 'first' : 'avg'}
3032+
]
3033+
}] :
3034+
[]
3035+
}]
3036+
};
3037+
}
3038+
3039+
var aggParcoords0Vals = [[1, 2, 3, 4], [4, 3, 2, 1]];
3040+
var aggParcoords1Vals = [[1, 2], [3, 2]];
3041+
var aggParcoords2Vals = [[2, 3], [4, 3]];
3042+
3043+
function checkCalcData(expectedCD) {
3044+
return function() {
3045+
expect(gd.calcdata.length).toBe(expectedCD.length);
3046+
expectedCD.forEach(function(expectedCDi, i) {
3047+
var cdi = gd.calcdata[i];
3048+
expect(cdi.length).toBe(expectedCDi.length, i);
3049+
expectedCDi.forEach(function(expectedij, j) {
3050+
expect(cdi[j]).toEqual(jasmine.objectContaining(expectedij));
3051+
});
3052+
});
3053+
};
3054+
}
3055+
3056+
function checkValues(expectedVals) {
3057+
return function() {
3058+
expect(gd._fullData.length).toBe(1);
3059+
var dims = gd._fullData[0].dimensions;
3060+
expect(dims.length).toBe(expectedVals.length);
3061+
expectedVals.forEach(function(expected, i) {
3062+
expect(dims[i].values).toEqual(expected);
3063+
});
3064+
};
3065+
}
3066+
3067+
function reactWith(fig) {
3068+
return function() { return Plotly.react(gd, fig); };
3069+
}
3070+
3071+
it('can change pie aggregations', function(done) {
3072+
Plotly.newPlot(gd, aggregatedPie(1))
3073+
.then(checkCalcData(aggPie1CD))
3074+
3075+
.then(reactWith(aggregatedPie(2)))
3076+
.then(checkCalcData(aggPie2CD))
3077+
3078+
.then(reactWith(aggregatedPie(1)))
3079+
.then(checkCalcData(aggPie1CD))
3080+
.catch(failTest)
3081+
.then(done);
3082+
});
3083+
3084+
it('can change scatter aggregations', function(done) {
3085+
Plotly.newPlot(gd, aggregatedScatter(1))
3086+
.then(checkCalcData(aggScatter1CD))
3087+
3088+
.then(reactWith(aggregatedScatter(2)))
3089+
.then(checkCalcData(aggScatter2CD))
3090+
3091+
.then(reactWith(aggregatedScatter(1)))
3092+
.then(checkCalcData(aggScatter1CD))
3093+
.catch(failTest)
3094+
.then(done);
3095+
});
3096+
3097+
it('can change parcoords aggregations', function(done) {
3098+
Plotly.newPlot(gd, aggregatedParcoords(0))
3099+
.then(checkValues(aggParcoords0Vals))
3100+
3101+
.then(reactWith(aggregatedParcoords(1)))
3102+
.then(checkValues(aggParcoords1Vals))
3103+
3104+
.then(reactWith(aggregatedParcoords(2)))
3105+
.then(checkValues(aggParcoords2Vals))
3106+
3107+
.then(reactWith(aggregatedParcoords(0)))
3108+
.then(checkValues(aggParcoords0Vals))
3109+
3110+
.catch(failTest)
3111+
.then(done);
3112+
});
3113+
3114+
it('can change type with aggregations', function(done) {
3115+
Plotly.newPlot(gd, aggregatedScatter(1))
3116+
.then(checkCalcData(aggScatter1CD))
3117+
3118+
.then(reactWith(aggregatedPie(1)))
3119+
.then(checkCalcData(aggPie1CD))
3120+
3121+
.then(reactWith(aggregatedParcoords(1)))
3122+
.then(checkValues(aggParcoords1Vals))
3123+
3124+
.then(reactWith(aggregatedScatter(1)))
3125+
.then(checkCalcData(aggScatter1CD))
3126+
3127+
.then(reactWith(aggregatedParcoords(2)))
3128+
.then(checkValues(aggParcoords2Vals))
3129+
3130+
.then(reactWith(aggregatedPie(2)))
3131+
.then(checkCalcData(aggPie2CD))
3132+
3133+
.then(reactWith(aggregatedScatter(2)))
3134+
.then(checkCalcData(aggScatter2CD))
3135+
3136+
.then(reactWith(aggregatedParcoords(0)))
3137+
.then(checkValues(aggParcoords0Vals))
3138+
.catch(failTest)
3139+
.then(done);
3140+
});
3141+
29583142
it('can change frames without redrawing', function(done) {
29593143
var data = [{y: [1, 2, 3]}];
29603144
var layout = {};

0 commit comments

Comments
 (0)