Skip to content

Commit 2d379f7

Browse files
antoinergantoinerg
committed
sort categories by values: improve code style
1 parent 9c09d8c commit 2d379f7

File tree

4 files changed

+55
-84
lines changed

4 files changed

+55
-84
lines changed

‎src/plots/plots.js‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2877,13 +2877,14 @@ function sortAxisCategoriesByValue(axList, gd) {
28772877
// Collect values across traces
28782878
for(j = 0; j < ax._traceIndices.length; j++) {
28792879
var traceIndex = ax._traceIndices[j];
2880-
var fullData = gd._fullData[traceIndex];
2880+
var fullTrace = gd._fullData[traceIndex];
2881+
var axLetter = ax._id.charAt(0);
28812882

28822883
// Skip over invisible traces
2883-
if(fullData.visible !== true) continue;
2884+
if(fullTrace.visible !== true) continue;
28842885

2885-
var type = fullData.type;
2886-
if(type === 'histogram') delete fullData._autoBinFinished;
2886+
var type = fullTrace.type;
2887+
if(Registry.traceIs(fullTrace, 'histogram')) delete fullTrace._autoBinFinished;
28872888

28882889
var cd = gd.calcdata[traceIndex];
28892890
for(k = 0; k < cd.length; k++) {
@@ -2893,10 +2894,10 @@ function sortAxisCategoriesByValue(axList, gd) {
28932894
// If `splom`, collect values across dimensions
28942895
if(type === 'splom') {
28952896
// Find which dimension the current axis is representing
2896-
var currentDimensionIndex = cdi.trace[ax._id.charAt(0) + 'axes'].indexOf(ax._id);
2897+
var currentDimensionIndex = cdi.trace[axLetter + 'axes'].indexOf(ax._id);
28972898

28982899
// Apply logic to associated x axis
2899-
if(ax._id.charAt(0) === 'y') {
2900+
if(axLetter === 'y') {
29002901
var associatedXAxis = ax._id.split('');
29012902
associatedXAxis[0] = 'x';
29022903
associatedXAxis = associatedXAxis.join('');
@@ -2918,13 +2919,13 @@ function sortAxisCategoriesByValue(axList, gd) {
29182919
// If `scattergl`, collect all values stashed under cdi.t
29192920
} else if(type === 'scattergl') {
29202921
for(l = 0; l < cdi.t.x.length; l++) {
2921-
if(ax._id.charAt(0) === 'x') {
2922+
if(axLetter === 'x') {
29222923
cat = cdi.t.x[l];
29232924
catIndex = cat;
29242925
value = cdi.t.y[l];
29252926
}
29262927

2927-
if(ax._id.charAt(0) === 'y') {
2928+
if(axLetter === 'y') {
29282929
cat = cdi.t.y[l];
29292930
catIndex = cat;
29302931
value = cdi.t.x[l];
@@ -2938,10 +2939,10 @@ function sortAxisCategoriesByValue(axList, gd) {
29382939
}
29392940
// For all other 2d cartesian traces
29402941
} else {
2941-
if(ax._id.charAt(0) === 'x') {
2942+
if(axLetter === 'x') {
29422943
cat = cdi.p + 1 ? cdi.p : cdi.x;
29432944
value = cdi.s || cdi.v || cdi.y;
2944-
} else if(ax._id.charAt(0) === 'y') {
2945+
} else if(axLetter === 'y') {
29452946
cat = cdi.p + 1 ? cdi.p : cdi.y;
29462947
value = cdi.s || cdi.v || cdi.x;
29472948
}

‎src/traces/bar/calc.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ module.exports = function calc(gd, trace) {
3333

3434
// set position and size
3535
for(var i = 0; i < serieslen; i++) {
36-
cd[i] = { p: pos[i], s: size[i], v: size[i] };
36+
cd[i] = { p: pos[i], s: size[i] };
3737

3838
if(trace.ids) {
3939
cd[i].id = String(trace.ids[i]);

‎test/image/mocks/sort_by_value_matching_axes.json‎

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4,26 +4,12 @@
44
"orientation": "v",
55
"x": ["a", "b", "c"],
66
"y": [7, 2, 3],
7-
8-
"z": [
9-
[7, 2, 3],
10-
[0, 0, 0],
11-
[0, 0, 0]
12-
],
13-
"dimensions": [{
14-
"label": "DimensionA",
15-
"values": ["a", "b", "c"]
16-
}, {
17-
"label": "DimensionB",
18-
"values": [7, 2, 3]
19-
}]
207
}, {
218
"type": "bar",
229
"x": ["a", "b", "c"],
2310
"y": [10, 20, 30],
2411
"yaxis": "y2",
2512
"xaxis": "x2"
26-
2713
}],
2814
"layout": {
2915
"xaxis": {

‎test/jasmine/tests/calcdata_test.js‎

Lines changed: 43 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -898,10 +898,10 @@ describe('calculated data and points', function() {
898898
// oneOrientationTraces are traces for which swapping x/y is not supported
899899
var oneOrientationTraces = ['ohlc', 'candlestick'];
900900

901-
function makeData(type, a, b, axId) {
901+
function makeData(type, a, b, axName) {
902902
var input = [a, b];
903-
var cat = input[axId === 'yaxis' ? 1 : 0];
904-
var data = input[axId === 'yaxis' ? 0 : 1];
903+
var cat = input[axName === 'yaxis' ? 1 : 0];
904+
var data = input[axName === 'yaxis' ? 0 : 1];
905905

906906
var measure = [];
907907
for(j = 0; j < data.length; j++) {
@@ -914,19 +914,19 @@ describe('calculated data and points', function() {
914914
z[j][k] = 0;
915915
}
916916
}
917-
if(axId === 'xaxis') {
917+
if(axName === 'xaxis') {
918918
for(j = 0; j < b.length; j++) {
919919
z[0][j] = b[j];
920920
}
921921
}
922-
if(axId === 'yaxis') {
922+
if(axName === 'yaxis') {
923923
for(j = 0; j < b.length; j++) {
924924
z[j][0] = b[j];
925925
}
926926
}
927927

928928
return Lib.extendDeep({}, {
929-
orientation: axId === 'yaxis' ? 'h' : 'v',
929+
orientation: axName === 'yaxis' ? 'h' : 'v',
930930
type: type,
931931
x: cat,
932932
a: cat,
@@ -959,124 +959,108 @@ describe('calculated data and points', function() {
959959
}
960960

961961
supportedCartesianTraces.forEach(function(trace) {
962-
['xaxis', 'yaxis'].forEach(function(axId) {
963-
if(axId === 'yaxis' && oneOrientationTraces.indexOf(trace.type) !== -1) return;
964-
['value ascending', 'value descending'].forEach(function(categoryorder) {
965-
it('sorts ' + axId + ' by ' + categoryorder + 'for trace type ' + trace.type, function(done) {
966-
var data = [7, 2, 3];
967-
var baseMock = { data: [makeData(trace.type, cat, data, axId)], layout: {}};
968-
var mock = Lib.extendDeep({}, baseMock);
969-
mock.layout[axId] = { type: 'category', categoryorder: categoryorder};
970-
971-
// Set ordering
972-
var finalOrder = ['b', 'c', 'a'];
973-
if(categoryorder === 'value descending') finalOrder.reverse();
974-
975-
if(trace.type.match(/histogram/)) {
976-
mock.data[0][axId === 'yaxis' ? 'y' : 'x'].push('a');
977-
mock.data[0][axId === 'yaxis' ? 'x' : 'y'].push(7);
978-
}
979-
980-
Plotly.newPlot(gd, mock)
981-
.then(function(gd) {
982-
expect(gd._fullLayout[trace.type === 'splom' ? 'xaxis' : axId]._categories).toEqual(finalOrder, 'for trace ' + trace.type);
983-
})
984-
.catch(failTest)
985-
.then(done);
986-
});
987-
});
962+
['xaxis', 'yaxis'].forEach(function(axName) {
963+
if(axName === 'yaxis' && oneOrientationTraces.indexOf(trace.type) !== -1) return;
988964

989-
function checkAggregatedValue(baseMock, expectedAgg, done) {
965+
function checkAggregatedValue(baseMock, expectedAgg, finalOrder, done) {
990966
var mock = Lib.extendDeep({}, baseMock);
991967

992968
if(mock.data[0].type.match(/histogram/)) {
993969
for(i = 0; i < mock.data.length; i++) {
994-
mock.data[i][axId === 'yaxis' ? 'y' : 'x'].push('a');
995-
mock.data[i][axId === 'yaxis' ? 'x' : 'y'].push(7);
970+
mock.data[i][axName === 'yaxis' ? 'y' : 'x'].push('a');
971+
mock.data[i][axName === 'yaxis' ? 'x' : 'y'].push(7);
996972
}
997973
}
998974

999975
Plotly.newPlot(gd, mock)
1000976
.then(function(gd) {
1001-
var agg = gd._fullLayout[mock.data[0].type === 'splom' ? 'xaxis' : axId]._categoriesAggregatedValue.sort(function(a, b) {
977+
var agg = gd._fullLayout[trace.type === 'splom' ? 'xaxis' : axName]._categoriesAggregatedValue.sort(function(a, b) {
1002978
return a[0] > b[0];
1003979
});
1004-
expect(agg).toEqual(expectedAgg, 'wrong aggregation for ' + axId);
980+
expect(agg).toEqual(expectedAgg, 'wrong aggregation for ' + axName);
981+
982+
if(finalOrder) {
983+
expect(gd._fullLayout[trace.type === 'splom' ? 'xaxis' : axName]._categories).toEqual(finalOrder, 'for trace ' + trace.type);
984+
}
1005985
})
1006986
.catch(failTest)
1007987
.then(done);
1008988
}
1009989

1010-
it('retrieves values in trace type ' + trace.type, function(done) {
1011-
var data = [7, 2, 3];
1012-
var baseMock = {data: [makeData(trace.type, cat, data, axId)], layout: {}};
1013-
baseMock.layout[axId] = { type: 'category', categoryorder: 'value ascending'};
990+
['value ascending', 'value descending'].forEach(function(categoryorder) {
991+
it('sorts ' + axName + ' by ' + categoryorder + ' for trace type ' + trace.type, function(done) {
992+
var data = [7, 2, 3];
993+
var baseMock = {data: [makeData(trace.type, cat, data, axName)], layout: {}};
994+
baseMock.layout[axName] = { type: 'category', categoryorder: categoryorder};
1014995

1015-
var expectedAgg = [['a', 7], ['b', 2], ['c', 3]];
1016-
if(trace.type === 'ohlc' || trace.type === 'candlestick') expectedAgg = [['a', 14], ['b', 4], ['c', 6]];
1017-
if(trace.type.match(/histogram/)) expectedAgg = [['a', 2], ['b', 1], ['c', 1]];
996+
// Set expectations
997+
var finalOrder = ['b', 'c', 'a'];
998+
if(categoryorder === 'value descending') finalOrder.reverse();
999+
var expectedAgg = [['a', 7], ['b', 2], ['c', 3]];
10181000

1019-
checkAggregatedValue(baseMock, expectedAgg, done);
1001+
if(trace.type === 'ohlc' || trace.type === 'candlestick') expectedAgg = [['a', 14], ['b', 4], ['c', 6]];
1002+
if(trace.type.match(/histogram/)) expectedAgg = [['a', 2], ['b', 1], ['c', 1]];
1003+
1004+
checkAggregatedValue(baseMock, expectedAgg, finalOrder, done);
1005+
});
10201006
});
10211007

10221008
it('sum values across traces of type ' + trace.type, function(done) {
10231009
var type = trace.type;
10241010
var data = [7, 2, 3];
10251011
var data2 = [5, 4, 2];
1026-
var baseMock = { data: [makeData(type, cat, data, axId), makeData(type, cat, data2, axId)], layout: {}};
1027-
baseMock.layout[axId] = { type: 'category', categoryorder: 'value ascending'};
1012+
var baseMock = { data: [makeData(type, cat, data, axName), makeData(type, cat, data2, axName)], layout: {}};
1013+
baseMock.layout[axName] = { type: 'category', categoryorder: 'value ascending'};
10281014

10291015
var expectedAgg = [['a', data[0] + data2[0]], ['b', data[1] + data2[1]], ['c', data[2] + data2[2]]];
10301016
if(type === 'ohlc' || type === 'candlestick') expectedAgg = [['a', 2 * expectedAgg[0][1]], ['b', 2 * expectedAgg[1][1]], ['c', 2 * expectedAgg[2][1]]];
10311017
if(type.match(/histogram/)) expectedAgg = [['a', 4], ['b', 2], ['c', 2]];
10321018

1033-
checkAggregatedValue(baseMock, expectedAgg, done);
1019+
checkAggregatedValue(baseMock, expectedAgg, false, done);
10341020
});
10351021

10361022
it('ignores values from traces that are not visible ' + trace.type, function(done) {
10371023
var type = trace.type;
10381024
var data = [7, 2, 3];
10391025
var data2 = [5, 4, 2];
1040-
var baseMock = { data: [makeData(type, cat, data, axId), makeData(type, cat, data2, axId)], layout: {}};
1041-
baseMock.layout[axId] = { type: 'category', categoryorder: 'value ascending'};
1026+
var baseMock = { data: [makeData(type, cat, data, axName), makeData(type, cat, data2, axName)], layout: {}};
1027+
baseMock.layout[axName] = { type: 'category', categoryorder: 'value ascending'};
10421028

10431029
// Hide second trace
10441030
baseMock.data[1].visible = 'legendonly';
10451031
var expectedAgg = [['a', data[0]], ['b', data[1]], ['c', data[2]]];
10461032
if(type === 'ohlc' || type === 'candlestick') expectedAgg = [['a', 2 * expectedAgg[0][1]], ['b', 2 * expectedAgg[1][1]], ['c', 2 * expectedAgg[2][1]]];
10471033
if(type.match(/histogram/)) expectedAgg = [['a', 2], ['b', 1], ['c', 1]];
10481034

1049-
checkAggregatedValue(baseMock, expectedAgg, done);
1035+
checkAggregatedValue(baseMock, expectedAgg, false, done);
10501036
});
10511037

10521038
it('finds the minimum value per category across traces of type ' + trace.type, function(done) {
10531039
var type = trace.type;
10541040
var data = [7, 2, 3];
10551041
var data2 = [5, 4, 2];
1056-
var baseMock = { data: [makeData(type, cat, data, axId), makeData(type, cat, data2, axId)], layout: {}};
1057-
baseMock.layout[axId] = { type: 'category', categoryorder: 'min ascending'};
1042+
var baseMock = { data: [makeData(type, cat, data, axName), makeData(type, cat, data2, axName)], layout: {}};
1043+
baseMock.layout[axName] = { type: 'category', categoryorder: 'min ascending'};
10581044

10591045
var expectedAgg = [['a', Math.min(data[0], data2[0])], ['b', Math.min(data[1], data2[1])], ['c', Math.min(data[2], data2[2])]];
1060-
// if(type === 'ohlc' || type === 'candlestick') expectedAgg = [['a', expectedAgg[0][1]], ['b', expectedAgg[1][1]], ['c', expectedAgg[2][1]]];
10611046
if(trace.categories.indexOf('2dMap') !== -1) expectedAgg = [['a', 0], ['b', 0], ['c', 0]];
10621047
if(type === 'histogram') expectedAgg = [['a', 2], ['b', 1], ['c', 1]];
10631048

1064-
checkAggregatedValue(baseMock, expectedAgg, done);
1049+
checkAggregatedValue(baseMock, expectedAgg, false, done);
10651050
});
10661051

10671052
it('finds the maximum value per category across traces of type ' + trace.type, function(done) {
10681053
var type = trace.type;
10691054
var data = [7, 2, 3];
10701055
var data2 = [5, 4, 2];
1071-
var baseMock = { data: [makeData(type, cat, data, axId), makeData(type, cat, data2, axId)], layout: {}};
1072-
baseMock.layout[axId] = { type: 'category', categoryorder: 'max ascending'};
1056+
var baseMock = { data: [makeData(type, cat, data, axName), makeData(type, cat, data2, axName)], layout: {}};
1057+
baseMock.layout[axName] = { type: 'category', categoryorder: 'max ascending'};
10731058

10741059
var expectedAgg = [['a', Math.max(data[0], data2[0])], ['b', Math.max(data[1], data2[1])], ['c', Math.max(data[2], data2[2])]];
10751060
if(type === 'ohlc' || type === 'candlestick') expectedAgg = [['a', expectedAgg[0][1]], ['b', expectedAgg[1][1]], ['c', expectedAgg[2][1]]];
1076-
// if(trace.categories.indexOf('2dMap') !== -1) expectedAgg = [['a', 0], ['b', 0], ['c', 0]];
10771061
if(type.match(/histogram/)) expectedAgg = [['a', 2], ['b', 1], ['c', 1]];
10781062

1079-
checkAggregatedValue(baseMock, expectedAgg, done);
1063+
checkAggregatedValue(baseMock, expectedAgg, false, done);
10801064
});
10811065
});
10821066
});

0 commit comments

Comments
 (0)