Skip to content

Commit a86ca58

Browse files
committed
fix and 🔒 bug with impliedEdits and groupby transform
1 parent 3a70fee commit a86ca58

File tree

2 files changed

+51
-17
lines changed

2 files changed

+51
-17
lines changed

src/plot_api/plot_api.js

+11-1
Original file line numberDiff line numberDiff line change
@@ -1420,6 +1420,16 @@ function _restyle(gd, aobj, traces) {
14201420

14211421
function rangeAttr(axName) { return 'LAYOUT' + axName + '.range'; }
14221422

1423+
function getFullTrace(traceIndex) {
1424+
// usually fullData maps 1:1 onto data, but with groupby transforms
1425+
// the fullData index can be greater. Take the *first* matching trace.
1426+
for(var j = traceIndex; j < fullData.length; j++) {
1427+
if(fullData[j]._input === data[traceIndex]) return fullData[j];
1428+
}
1429+
// should never get here - and if we *do* it should cause an error
1430+
// later on undefined fullTrace is passed to nestedProperty.
1431+
}
1432+
14231433
// for attrs that interact (like scales & autoscales), save the
14241434
// old vals before making the change
14251435
// val=undefined will not set a value, just record what the value was.
@@ -1507,7 +1517,7 @@ function _restyle(gd, aobj, traces) {
15071517
undoit[ai] = a0();
15081518
for(i = 0; i < traces.length; i++) {
15091519
cont = data[traces[i]];
1510-
contFull = fullData[traces[i]];
1520+
contFull = getFullTrace(traces[i]);
15111521
param = Lib.nestedProperty(cont, ai);
15121522
oldVal = param.get();
15131523
newVal = Array.isArray(vi) ? vi[i % vi.length] : vi;

test/jasmine/tests/plot_api_test.js

+40-16
Original file line numberDiff line numberDiff line change
@@ -1266,28 +1266,52 @@ describe('Test plot api', function() {
12661266
.then(done);
12671267
});
12681268

1269-
it('sets x/ytype scaled when editing heatmap x0/dx/y0/dy', function(done) {
1270-
var x0 = 3;
1271-
var dy = 5;
1272-
1273-
function check(scaled, msg) {
1274-
expect(gd.data[0].x0).negateIf(!scaled).toBe(x0, msg);
1275-
expect(gd.data[0].xtype).toBe(scaled ? 'scaled' : undefined, msg);
1276-
expect(gd.data[0].dy).negateIf(!scaled).toBe(dy, msg);
1277-
expect(gd.data[0].ytype).toBe(scaled ? 'scaled' : undefined, msg);
1278-
}
1269+
function checkScaling(xyType, xyTypeIn, iIn, iOut) {
1270+
expect(gd._fullData[iOut].xtype).toBe(xyType);
1271+
expect(gd._fullData[iOut].ytype).toBe(xyType);
1272+
expect(gd.data[iIn].xtype).toBe(xyTypeIn);
1273+
expect(gd.data[iIn].ytype).toBe(xyTypeIn);
1274+
}
12791275

1280-
Plotly.plot(gd, [{x: [1, 2, 4], y: [2, 3, 5], z: [[1, 2], [3, 4]], type: 'heatmap'}])
1276+
it('sets heatmap xtype/ytype when you edit x/y data or scaling params', function(done) {
1277+
Plotly.plot(gd, [{type: 'heatmap', z: [[0, 1], [2, 3]]}])
12811278
.then(function() {
1282-
check(false, 'initial');
1283-
return Plotly.restyle(gd, {x0: x0, dy: dy});
1279+
// TODO would probably be better to actively default to 'array' here...
1280+
checkScaling(undefined, undefined, 0, 0);
1281+
return Plotly.restyle(gd, {x: [[2, 4]], y: [[3, 5]]}, [0]);
12841282
})
12851283
.then(function() {
1286-
check(true, 'set x0 & dy');
1287-
return Queue.undo(gd);
1284+
checkScaling('array', 'array', 0, 0);
1285+
return Plotly.restyle(gd, {x0: 1, dy: 3}, [0]);
12881286
})
12891287
.then(function() {
1290-
check(false, 'undo');
1288+
checkScaling('scaled', 'scaled', 0, 0);
1289+
})
1290+
.catch(failTest)
1291+
.then(done);
1292+
});
1293+
1294+
it('sets heatmap xtype/ytype even when data/fullData indices mismatch', function(done) {
1295+
Plotly.plot(gd, [
1296+
{
1297+
// importantly, this is NOT a heatmap trace, so _fullData[1]
1298+
// will not have the same attributes as data[1]
1299+
x: [1, -1, -2, 0],
1300+
y: [1, 2, 3, 1],
1301+
transforms: [{type: 'groupby', groups: ['a', 'b', 'a', 'b']}]
1302+
},
1303+
{type: 'heatmap', z: [[0, 1], [2, 3]]}
1304+
])
1305+
.then(function() {
1306+
checkScaling(undefined, undefined, 1, 2);
1307+
return Plotly.restyle(gd, {x: [[2, 4]], y: [[3, 5]]}, [1]);
1308+
})
1309+
.then(function() {
1310+
checkScaling('array', 'array', 1, 2);
1311+
return Plotly.restyle(gd, {x0: 1, dy: 3}, [1]);
1312+
})
1313+
.then(function() {
1314+
checkScaling('scaled', 'scaled', 1, 2);
12911315
})
12921316
.catch(failTest)
12931317
.then(done);

0 commit comments

Comments
 (0)