Skip to content

Commit ada31b5

Browse files
etpinardetpinard
committed
robustify 'find axis' logic:
- make sure that graphs with only orphan cartesian are considered 'cartesian' - make sure that axes associated with GL2D traces aren't considered 'cartesian' - by making layout with { xaxis: {}, yaxis: {} } a 'hasCartesian' plot, deleting the last cartesian trace retains the axes.
1 parent 07ca162 commit ada31b5

File tree

2 files changed

+128
-29
lines changed

2 files changed

+128
-29
lines changed

Diff for: ‎src/plots/cartesian/layout_defaults.js

+58-25
Original file line numberDiff line numberDiff line change
@@ -20,28 +20,36 @@ var axisIds = require('./axis_ids');
2020

2121

2222
module.exports = function supplyLayoutDefaults(layoutIn, layoutOut, fullData) {
23-
// get the full list of axes already defined
2423
var layoutKeys = Object.keys(layoutIn),
25-
xaList = [],
26-
yaList = [],
24+
xaListCartesian = [],
25+
yaListCartesian = [],
26+
xaListGl2d = [],
27+
yaListGl2d = [],
2728
outerTicks = {},
2829
noGrids = {},
2930
i;
3031

31-
for(i = 0; i < layoutKeys.length; i++) {
32-
var key = layoutKeys[i];
33-
if(constants.xAxisMatch.test(key)) xaList.push(key);
34-
else if(constants.yAxisMatch.test(key)) yaList.push(key);
35-
}
36-
32+
// look for axes in the data
3733
for(i = 0; i < fullData.length; i++) {
38-
var trace = fullData[i],
39-
xaName = axisIds.id2name(trace.xaxis),
34+
var trace = fullData[i];
35+
var listX, listY;
36+
37+
if(Plots.traceIs(trace, 'cartesian')) {
38+
listX = xaListCartesian;
39+
listY = yaListCartesian;
40+
}
41+
else if(Plots.traceIs(trace, 'gl2d')) {
42+
listX = xaListGl2d;
43+
listY = yaListGl2d;
44+
}
45+
else continue;
46+
47+
var xaName = axisIds.id2name(trace.xaxis),
4048
yaName = axisIds.id2name(trace.yaxis);
4149

4250
// add axes implied by traces
43-
if(xaName && xaList.indexOf(xaName) === -1) xaList.push(xaName);
44-
if(yaName && yaList.indexOf(yaName) === -1) yaList.push(yaName);
51+
if(xaName && listX.indexOf(xaName) === -1) listX.push(xaName);
52+
if(yaName && listY.indexOf(yaName) === -1) listY.push(yaName);
4553

4654
// check for default formatting tweaks
4755
if(Plots.traceIs(trace, '2dMap')) {
@@ -55,22 +63,47 @@ module.exports = function supplyLayoutDefaults(layoutIn, layoutOut, fullData) {
5563
}
5664
}
5765

58-
function axSort(a,b) {
59-
var aNum = Number(a.substr(5)||1),
60-
bNum = Number(b.substr(5)||1);
61-
return aNum - bNum;
66+
// N.B. Ignore orphan axes (i.e. axes that have no data attached to them)
67+
// if gl3d or geo is present on graph. This is retain backward compatible.
68+
//
69+
// TODO drop this in version 2.0
70+
var ignoreOrphan = (layoutOut._hasGL3D || layoutOut._hasGeo);
71+
72+
if(!ignoreOrphan) {
73+
for(i = 0; i < layoutKeys.length; i++) {
74+
var key = layoutKeys[i];
75+
76+
// orphan layout axes are considered cartesian subplots
77+
78+
if(xaListGl2d.indexOf(key) === -1 &&
79+
xaListCartesian.indexOf(key) === -1 &&
80+
constants.xAxisMatch.test(key)) {
81+
xaListCartesian.push(key);
82+
}
83+
else if(yaListGl2d.indexOf(key) === -1 &&
84+
yaListCartesian.indexOf(key) === -1 &&
85+
constants.yAxisMatch.test(key)) {
86+
yaListCartesian.push(key);
87+
}
88+
}
6289
}
6390

64-
if(layoutOut._hasCartesian || layoutOut._hasGL2D || !fullData.length) {
65-
// make sure there's at least one of each and lists are sorted
66-
if(!xaList.length) xaList = ['xaxis'];
67-
else xaList.sort(axSort);
91+
// make sure that plots with orphan cartesian axes
92+
// are considered 'cartesian'
93+
if(xaListCartesian.length && yaListCartesian.length) {
94+
layoutOut._hasCartesian = true;
95+
}
6896

69-
if(!yaList.length) yaList = ['yaxis'];
70-
else yaList.sort(axSort);
97+
function axSort(a, b) {
98+
var aNum = Number(a.substr(5) || 1),
99+
bNum = Number(b.substr(5) || 1);
100+
return aNum - bNum;
71101
}
72102

73-
xaList.concat(yaList).forEach(function(axName){
103+
var xaList = xaListCartesian.concat(xaListGl2d).sort(axSort),
104+
yaList = yaListCartesian.concat(yaListGl2d).sort(axSort);
105+
106+
xaList.concat(yaList).forEach(function(axName) {
74107
var axLetter = axName.charAt(0),
75108
axLayoutIn = layoutIn[axName] || {},
76109
axLayoutOut = {},
@@ -100,7 +133,7 @@ module.exports = function supplyLayoutDefaults(layoutIn, layoutOut, fullData) {
100133

101134
// so we don't have to repeat autotype unnecessarily,
102135
// copy an autotype back to layoutIn
103-
if(!layoutIn[axName] && axLayoutIn.type!=='-') {
136+
if(!layoutIn[axName] && axLayoutIn.type !== '-') {
104137
layoutIn[axName] = {type: axLayoutIn.type};
105138
}
106139

Diff for: ‎test/jasmine/tests/axes_test.js

+70-4
Original file line numberDiff line numberDiff line change
@@ -167,9 +167,12 @@ describe('Test axes', function() {
167167
});
168168

169169
describe('supplyLayoutDefaults', function() {
170-
var layoutIn = {},
171-
layoutOut = {},
170+
var layoutIn, layoutOut, fullData;
171+
172+
beforeEach(function() {
173+
layoutOut = {};
172174
fullData = [];
175+
});
173176

174177
var supplyLayoutDefaults = Axes.supplyLayoutDefaults;
175178

@@ -196,7 +199,7 @@ describe('Test axes', function() {
196199

197200
it('should set linewidth to default if linecolor is supplied and valid', function() {
198201
layoutIn = {
199-
xaxis: {linecolor:'black'}
202+
xaxis: { linecolor: 'black' }
200203
};
201204
supplyLayoutDefaults(layoutIn, layoutOut, fullData);
202205
expect(layoutOut.xaxis.linecolor).toBe('black');
@@ -205,7 +208,7 @@ describe('Test axes', function() {
205208

206209
it('should set linecolor to default if linewidth is supplied and valid', function() {
207210
layoutIn = {
208-
yaxis: {linewidth:2}
211+
yaxis: { linewidth: 2 }
209212
};
210213
supplyLayoutDefaults(layoutIn, layoutOut, fullData);
211214
expect(layoutOut.yaxis.linewidth).toBe(2);
@@ -253,6 +256,69 @@ describe('Test axes', function() {
253256
expect(layoutOut.xaxis.zerolinewidth).toBe(undefined);
254257
expect(layoutOut.xaxis.zerolinecolor).toBe(undefined);
255258
});
259+
260+
it('should detect orphan axes (lone axes case)', function() {
261+
layoutIn = {
262+
xaxis: {},
263+
yaxis: {}
264+
};
265+
fullData = [];
266+
267+
supplyLayoutDefaults(layoutIn, layoutOut, fullData);
268+
expect(layoutOut._hasCartesian).toBe(true);
269+
});
270+
271+
it('should detect orphan axes (gl2d trace conflict case)', function() {
272+
layoutIn = {
273+
xaxis: {},
274+
yaxis: {}
275+
};
276+
fullData = [{
277+
type: 'scattergl',
278+
xaxis: 'x',
279+
yaxis: 'y'
280+
}];
281+
282+
supplyLayoutDefaults(layoutIn, layoutOut, fullData);
283+
expect(layoutOut._hasCartesian).toBe(undefined);
284+
});
285+
286+
it('should detect orphan axes (gl2d + cartesian case)', function() {
287+
layoutIn = {
288+
xaxis2: {},
289+
yaxis2: {}
290+
};
291+
fullData = [{
292+
type: 'scattergl',
293+
xaxis: 'x',
294+
yaxis: 'y'
295+
}];
296+
297+
supplyLayoutDefaults(layoutIn, layoutOut, fullData);
298+
expect(layoutOut._hasCartesian).toBe(true);
299+
});
300+
301+
it('should detect orphan axes (gl3d present case)', function() {
302+
layoutIn = {
303+
xaxis: {},
304+
yaxis: {}
305+
};
306+
layoutOut._hasGL3D = true;
307+
308+
supplyLayoutDefaults(layoutIn, layoutOut, fullData);
309+
expect(layoutOut._hasCartesian).toBe(undefined);
310+
});
311+
312+
it('should detect orphan axes (gl3d present case)', function() {
313+
layoutIn = {
314+
xaxis: {},
315+
yaxis: {}
316+
};
317+
layoutOut._hasGeo = true;
318+
319+
supplyLayoutDefaults(layoutIn, layoutOut, fullData);
320+
expect(layoutOut._hasCartesian).toBe(undefined);
321+
});
256322
});
257323

258324
describe('handleTickDefaults', function() {

0 commit comments

Comments
 (0)