Skip to content

Commit a285aeb

Browse files
etpinardetpinard
authored
Merge pull request #3072 from plotly/fix-noCI-tests-on-master
Fix parcats + Plotly.react bug
2 parents 97cb217 + 24af02a commit a285aeb

File tree

8 files changed

+62
-66
lines changed

8 files changed

+62
-66
lines changed

Diff for: ‎src/traces/parcats/calc.js

+32-50
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,6 @@ var filterUnique = require('../../lib/filter_unique.js');
1717
var Drawing = require('../../components/drawing');
1818
var Lib = require('../../lib');
1919

20-
21-
function visible(dimension) { return !('visible' in dimension) || dimension.visible; }
22-
23-
// Exports
24-
// =======
2520
/**
2621
* Create a wrapped ParcatsModel object from trace
2722
*
@@ -31,24 +26,18 @@ function visible(dimension) { return !('visible' in dimension) || dimension.visi
3126
* @return {Array.<ParcatsModel>}
3227
*/
3328
module.exports = function calc(gd, trace) {
29+
var visibleDims = Lib.filterVisible(trace.dimensions);
3430

35-
// Process inputs
36-
// --------------
37-
if(trace.dimensions.filter(visible).length === 0) {
38-
// No visible dimensions specified. Nothing to compute
39-
return [];
40-
}
31+
if(visibleDims.length === 0) return [];
4132

42-
// Compute unique information
43-
// --------------------------
44-
// UniqueInfo per dimension
45-
var uniqueInfoDims = trace.dimensions.filter(visible).map(function(dim) {
33+
var uniqueInfoDims = visibleDims.map(function(dim) {
4634
var categoryValues;
4735
if(dim.categoryorder === 'trace') {
4836
// Use order of first occurrence in trace
4937
categoryValues = null;
5038
} else if(dim.categoryorder === 'array') {
51-
// Use categories specified in `categoryarray` first, then add extra to the end in trace order
39+
// Use categories specified in `categoryarray` first,
40+
// then add extra to the end in trace order
5241
categoryValues = dim.categoryarray;
5342
} else {
5443
// Get all categories up front so we can order them
@@ -61,8 +50,6 @@ module.exports = function calc(gd, trace) {
6150
return getUniqueInfo(dim.values, categoryValues);
6251
});
6352

64-
// Process counts
65-
// --------------
6653
var counts,
6754
count,
6855
totalCount;
@@ -72,13 +59,9 @@ module.exports = function calc(gd, trace) {
7259
counts = [trace.counts];
7360
}
7461

75-
// Validate dimension display order
76-
// --------------------------------
77-
validateDimensionDisplayInds(trace);
62+
validateDimensionDisplayInds(visibleDims);
7863

79-
// Validate category display order
80-
// -------------------------------
81-
trace.dimensions.filter(visible).forEach(function(dim, dimInd) {
64+
visibleDims.forEach(function(dim, dimInd) {
8265
validateCategoryProperties(dim, uniqueInfoDims[dimInd]);
8366
});
8467

@@ -111,7 +94,7 @@ module.exports = function calc(gd, trace) {
11194

11295
// Number of values and counts
11396
// ---------------------------
114-
var numValues = trace.dimensions.filter(visible)[0].values.length;
97+
var numValues = visibleDims[0].values.length;
11598

11699
// Build path info
117100
// ---------------
@@ -155,12 +138,8 @@ module.exports = function calc(gd, trace) {
155138
updatePathModel(pathModels[pathKey], valueInd, count);
156139
}
157140

158-
// Build categories info
159-
// ---------------------
160-
161-
// Array of DimensionModel objects
162-
var dimensionModels = trace.dimensions.filter(visible).map(function(di, i) {
163-
return createDimensionModel(i, di._index, di.displayindex, di.label, totalCount);
141+
var dimensionModels = visibleDims.map(function(di, i) {
142+
return createDimensionModel(i, di._index, di._displayindex, di.label, totalCount);
164143
});
165144

166145

@@ -174,8 +153,8 @@ module.exports = function calc(gd, trace) {
174153
var cats = dimensionModels[d].categories;
175154

176155
if(cats[catInd] === undefined) {
177-
var catValue = trace.dimensions[containerInd].categoryarray[catInd];
178-
var catLabel = trace.dimensions[containerInd].ticktext[catInd];
156+
var catValue = trace.dimensions[containerInd]._categoryarray[catInd];
157+
var catLabel = trace.dimensions[containerInd]._ticktext[catInd];
179158
cats[catInd] = createCategoryModel(d, catInd, catValue, catLabel);
180159
}
181160

@@ -216,7 +195,6 @@ module.exports = function calc(gd, trace) {
216195
*/
217196
function createParcatsModel(dimensions, paths, count) {
218197
var maxCats = dimensions
219-
.filter(visible)
220198
.map(function(d) {return d.categories.length;})
221199
.reduce(function(v1, v2) {return Math.max(v1, v2);});
222200
return {dimensions: dimensions, paths: paths, trace: undefined, maxCats: maxCats, count: count};
@@ -456,43 +434,47 @@ function getUniqueInfo(values, uniqueValues) {
456434

457435
/**
458436
* Validate the requested display order for the dimensions.
459-
* If the display order is a permutation of 0 through dimensions.length - 1 then leave it alone. Otherwise, repalce
460-
* the display order with the dimension order
437+
* If the display order is a permutation of 0 through dimensions.length - 1, link to _displayindex
438+
* Otherwise, replace the display order with the dimension order
461439
* @param {Object} trace
462440
*/
463-
function validateDimensionDisplayInds(trace) {
464-
var displayInds = trace.dimensions.filter(visible).map(function(dim) {return dim.displayindex;});
465-
if(!isRangePermutation(displayInds)) {
466-
trace.dimensions.filter(visible).forEach(function(dim, i) {
467-
dim.displayindex = i;
468-
});
441+
function validateDimensionDisplayInds(visibleDims) {
442+
var displayInds = visibleDims.map(function(d) { return d.displayindex; });
443+
var i;
444+
445+
if(isRangePermutation(displayInds)) {
446+
for(i = 0; i < visibleDims.length; i++) {
447+
visibleDims[i]._displayindex = visibleDims[i].displayindex;
448+
}
449+
} else {
450+
for(i = 0; i < visibleDims.length; i++) {
451+
visibleDims[i]._displayindex = i;
452+
}
469453
}
470454
}
471455

472456

473457
/**
474-
* Validate the requested display order for the dimensions.
475-
* If the display order is a permutation of 0 through dimensions.length - 1 then leave it alone. Otherwise, repalce
476-
* the display order with the dimension order
458+
* Update category properties based on the unique values found for this dimension
477459
* @param {Object} dim
478460
* @param {UniqueInfo} uniqueInfoDim
479461
*/
480462
function validateCategoryProperties(dim, uniqueInfoDim) {
481463

482464
// Update categoryarray
483-
dim.categoryarray = uniqueInfoDim.uniqueValues;
465+
dim._categoryarray = uniqueInfoDim.uniqueValues;
484466

485467
// Handle ticktext
486468
if(dim.ticktext === null || dim.ticktext === undefined) {
487-
dim.ticktext = [];
469+
dim._ticktext = [];
488470
} else {
489471
// Shallow copy to avoid modifying input array
490-
dim.ticktext = dim.ticktext.slice();
472+
dim._ticktext = dim.ticktext.slice();
491473
}
492474

493475
// Extend ticktext with elements from uniqueInfoDim.uniqueValues
494-
for(var i = dim.ticktext.length; i < uniqueInfoDim.uniqueValues.length; i++) {
495-
dim.ticktext.push(uniqueInfoDim.uniqueValues[i]);
476+
for(var i = dim._ticktext.length; i < uniqueInfoDim.uniqueValues.length; i++) {
477+
dim._ticktext.push(uniqueInfoDim.uniqueValues[i]);
496478
}
497479
}
498480

Diff for: ‎test/image/baselines/parcats_bad-displayindex.png

46.9 KB
Loading

Diff for: ‎test/image/mocks/parcats_bad-displayindex.json

+17
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
{
2+
"data": [
3+
{"type": "parcats",
4+
"dimensions": [
5+
{"label": "One", "values": [1, 1, 2, 1, 2, 1, 1, 2, 1],
6+
"displayindex": 1, "categoryarray": [1, 2], "ticktext": ["One", "Two"]},
7+
{"label": "Two", "values": ["A", "B", "A", "B", "C", "C", "A", "B", "C"],
8+
"displayindex": 2, "categoryarray": ["B", "A", "C"]},
9+
{"label": "Three", "values": [11, 11, 11, 11, 11, 11, 11, 11, 11], "displayindex": 1}]}
10+
],
11+
"layout": {
12+
"height": 602,
13+
"width": 592,
14+
"margin": {
15+
"l": 40, "r": 40, "t": 50, "b": 40
16+
}}
17+
}

Diff for: ‎test/jasmine/assets/mock_lists.js

+1
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ var svgMockList = [
2727
['range_selector_style', require('@mocks/range_selector_style.json')],
2828
['range_slider_multiple', require('@mocks/range_slider_multiple.json')],
2929
['sankey_energy', require('@mocks/sankey_energy.json')],
30+
['parcats_bad-displayindex', require('@mocks/parcats_bad-displayindex.json')],
3031
['scattercarpet', require('@mocks/scattercarpet.json')],
3132
['shapes', require('@mocks/shapes.json')],
3233
['splom_iris', require('@mocks/splom_iris.json')],

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -363,7 +363,7 @@ describe('Drawing', function() {
363363
'width', 'left', 'right'
364364
].forEach(function(dim) {
365365
// give larger dimensions some extra tolerance
366-
var tol = Math.max(expected[dim] / 10, 3);
366+
var tol = Math.max(expected[dim] / 10, 3.5);
367367
expect(actual[dim]).toBeWithin(expected[dim], tol, dim);
368368
});
369369
}

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

-4
Original file line numberDiff line numberDiff line change
@@ -723,7 +723,6 @@ describe('Drag to reordered dimensions', function() {
723723

724724
Plotly.newPlot(gd, mock)
725725
.then(function() {
726-
console.log(gd.data);
727726
restyleCallback = jasmine.createSpy('restyleCallback');
728727
gd.on('plotly_restyle', restyleCallback);
729728

@@ -1229,9 +1228,6 @@ describe('Click events', function() {
12291228
/** @type {ParcatsViewModel} */
12301229
var parcatsViewModel = d3.select('g.trace.parcats').datum();
12311230

1232-
console.log(gd.data[0]);
1233-
console.log(parcatsViewModel.hoverinfoItems);
1234-
12351231
gd.on('plotly_click', function(data) {
12361232
clickData = data;
12371233
});

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

+7-7
Original file line numberDiff line numberDiff line change
@@ -198,8 +198,8 @@ describe('Pie traces:', function() {
198198
expect(title.size()).toBe(1);
199199
var titlePos = getClientPosition('g.titletext');
200200
var pieCenterPos = getClientPosition('g.trace');
201-
expect(Math.abs(titlePos[0] - pieCenterPos[0])).toBeLessThan(2);
202-
expect(Math.abs(titlePos[1] - pieCenterPos[1])).toBeLessThan(2);
201+
expect(Math.abs(titlePos[0] - pieCenterPos[0])).toBeLessThan(4);
202+
expect(Math.abs(titlePos[1] - pieCenterPos[1])).toBeLessThan(4);
203203
})
204204
.catch(failTest)
205205
.then(done);
@@ -229,15 +229,15 @@ describe('Pie traces:', function() {
229229
var pieBox = d3.select('g.trace').node().getBoundingClientRect();
230230
var radius = 0.1 * Math.min(pieBox.width / 2, pieBox.height / 2);
231231
var pieCenterPos = getClientPosition('g.trace');
232-
// unfortunately boundingClientRect is inaccurate and so we allow an error of 2
232+
// unfortunately boundingClientRect is inaccurate and so we allow an error of 3
233233
expect(_verifyPointInCircle(titleBox.left, titleBox.top, pieCenterPos, radius))
234-
.toBeLessThan(2);
234+
.toBeLessThan(3);
235235
expect(_verifyPointInCircle(titleBox.right, titleBox.top, pieCenterPos, radius))
236-
.toBeLessThan(2);
236+
.toBeLessThan(3);
237237
expect(_verifyPointInCircle(titleBox.left, titleBox.bottom, pieCenterPos, radius))
238-
.toBeLessThan(2);
238+
.toBeLessThan(3);
239239
expect(_verifyPointInCircle(titleBox.right, titleBox.bottom, pieCenterPos, radius))
240-
.toBeLessThan(2);
240+
.toBeLessThan(3);
241241
})
242242
.catch(failTest)
243243
.then(done);

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

+4-4
Original file line numberDiff line numberDiff line change
@@ -334,13 +334,13 @@ describe('sliders interactions', function() {
334334
d3.select(gd).selectAll('.slider-group').each(function(d, i) {
335335
var sliderBB = this.getBoundingClientRect();
336336
var gdBB = gd.getBoundingClientRect();
337+
337338
if(i === 0) {
338339
expect(sliderBB.left - gdBB.left)
339-
.toBeWithin(12, 3, 'left: ' + msg);
340-
}
341-
else {
340+
.toBeWithin(12, 5.1, 'left: ' + msg);
341+
} else {
342342
expect(gdBB.bottom - sliderBB.bottom)
343-
.toBeWithin(8, 3, 'bottom: ' + msg);
343+
.toBeWithin(8, 5.1, 'bottom: ' + msg);
344344
}
345345
});
346346
}

0 commit comments

Comments
 (0)