Skip to content

Commit c82ad2e

Browse files
archmojarchmoj
authored
Merge pull request #4783 from sightmachine/sort-order-with-null
Addressing transform sort order with null values
2 parents eca4b3b + d267337 commit c82ad2e

File tree

2 files changed

+83
-54
lines changed

2 files changed

+83
-54
lines changed

‎src/transforms/sort.js‎

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ var Lib = require('../lib');
1212
var Axes = require('../plots/cartesian/axes');
1313
var pointsAccessorFunction = require('./helpers').pointsAccessorFunction;
1414

15+
var BADNUM = require('../constants/numerical').BADNUM;
16+
1517
exports.moduleType = 'transform';
1618

1719
exports.name = 'sort';
@@ -136,8 +138,28 @@ function getIndices(opts, targetArray, d2c, len) {
136138
function getSortFunc(opts, d2c) {
137139
switch(opts.order) {
138140
case 'ascending':
139-
return function(a, b) { return d2c(a.v) - d2c(b.v); };
141+
return function(a, b) {
142+
var ac = d2c(a.v);
143+
var bc = d2c(b.v);
144+
if(ac === BADNUM) {
145+
return 1;
146+
}
147+
if(bc === BADNUM) {
148+
return -1;
149+
}
150+
return ac - bc;
151+
};
140152
case 'descending':
141-
return function(a, b) { return d2c(b.v) - d2c(a.v); };
153+
return function(a, b) {
154+
var ac = d2c(a.v);
155+
var bc = d2c(b.v);
156+
if(ac === BADNUM) {
157+
return 1;
158+
}
159+
if(bc === BADNUM) {
160+
return -1;
161+
}
162+
return bc - ac;
163+
};
142164
}
143165
}

‎test/jasmine/tests/transform_sort_test.js‎

Lines changed: 59 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -58,12 +58,12 @@ describe('Test sort transform defaults:', function() {
5858

5959
describe('Test sort transform calc:', function() {
6060
var base = {
61-
x: [-2, -1, -2, 0, 1, 3, 1],
62-
y: [1, 2, 3, 1, 2, 3, 1],
63-
ids: ['n0', 'n1', 'n2', 'z', 'p1', 'p2', 'p3'],
61+
x: [-2, -1, -2, 0, 1, 3, null, 1],
62+
y: [1, 2, 3, 1, 2, 3, 4, 1],
63+
ids: ['n0', 'n1', 'n2', 'z', 'p1', 'p2', 'n3', 'p3'],
6464
marker: {
65-
color: [0.1, 0.2, 0.3, 0.1, 0.2, 0.3, 0.4],
66-
size: [10, 20, 5, 1, 6, 0, 10]
65+
color: [0.1, 0.2, 0.3, 0.1, 0.2, 0.3, 0.4, 0.4],
66+
size: [10, 20, 5, 1, 6, 0, 3, 10]
6767
},
6868
transforms: [{ type: 'sort' }]
6969
};
@@ -89,21 +89,26 @@ describe('Test sort transform calc:', function() {
8989
}
9090

9191
it('should sort all array attributes (ascending case)', function() {
92-
var out = _transform([extend({})]);
92+
var out = _transform([extend({
93+
transforms: [{
94+
order: 'ascending'
95+
}]
96+
})]);
9397

94-
expect(out[0].x).toEqual([-2, -2, -1, 0, 1, 1, 3]);
95-
expect(out[0].y).toEqual([1, 3, 2, 1, 2, 1, 3]);
96-
expect(out[0].ids).toEqual(['n0', 'n2', 'n1', 'z', 'p1', 'p3', 'p2']);
97-
expect(out[0].marker.color).toEqual([0.1, 0.3, 0.2, 0.1, 0.2, 0.4, 0.3]);
98-
expect(out[0].marker.size).toEqual([10, 5, 20, 1, 6, 10, 0]);
98+
expect(out[0].x).toEqual([-2, -2, -1, 0, 1, 1, 3, null]);
99+
expect(out[0].y).toEqual([1, 3, 2, 1, 2, 1, 3, 4]);
100+
expect(out[0].ids).toEqual(['n0', 'n2', 'n1', 'z', 'p1', 'p3', 'p2', 'n3']);
101+
expect(out[0].marker.color).toEqual([0.1, 0.3, 0.2, 0.1, 0.2, 0.4, 0.3, 0.4]);
102+
expect(out[0].marker.size).toEqual([10, 5, 20, 1, 6, 10, 0, 3]);
99103
expect(out[0].transforms[0]._indexToPoints).toEqual({
100104
0: [0],
101105
1: [2],
102106
2: [1],
103107
3: [3],
104108
4: [4],
105-
5: [6],
106-
6: [5]
109+
5: [7],
110+
6: [5],
111+
7: [6],
107112
});
108113
});
109114

@@ -114,19 +119,20 @@ describe('Test sort transform calc:', function() {
114119
}]
115120
})]);
116121

117-
expect(out[0].x).toEqual([3, 1, 1, 0, -1, -2, -2]);
118-
expect(out[0].y).toEqual([3, 2, 1, 1, 2, 1, 3]);
119-
expect(out[0].ids).toEqual(['p2', 'p1', 'p3', 'z', 'n1', 'n0', 'n2']);
120-
expect(out[0].marker.color).toEqual([0.3, 0.2, 0.4, 0.1, 0.2, 0.1, 0.3]);
121-
expect(out[0].marker.size).toEqual([0, 6, 10, 1, 20, 10, 5]);
122+
expect(out[0].x).toEqual([3, 1, 1, 0, -1, -2, -2, null]);
123+
expect(out[0].y).toEqual([3, 2, 1, 1, 2, 1, 3, 4]);
124+
expect(out[0].ids).toEqual(['p2', 'p1', 'p3', 'z', 'n1', 'n0', 'n2', 'n3']);
125+
expect(out[0].marker.color).toEqual([0.3, 0.2, 0.4, 0.1, 0.2, 0.1, 0.3, 0.4]);
126+
expect(out[0].marker.size).toEqual([0, 6, 10, 1, 20, 10, 5, 3]);
122127
expect(out[0].transforms[0]._indexToPoints).toEqual({
123128
0: [5],
124129
1: [4],
125-
2: [6],
130+
2: [7],
126131
3: [3],
127132
4: [1],
128133
5: [0],
129-
6: [2]
134+
6: [2],
135+
7: [6]
130136
});
131137
});
132138

@@ -138,19 +144,20 @@ describe('Test sort transform calc:', function() {
138144
}]
139145
})]);
140146

141-
expect(out[0].x).toEqual([-1, -2, 1, 1, -2, 0, 3]);
142-
expect(out[0].y).toEqual([2, 1, 1, 2, 3, 1, 3]);
143-
expect(out[0].ids).toEqual(['n1', 'n0', 'p3', 'p1', 'n2', 'z', 'p2']);
144-
expect(out[0].marker.color).toEqual([0.2, 0.1, 0.4, 0.2, 0.3, 0.1, 0.3]);
145-
expect(out[0].marker.size).toEqual([20, 10, 10, 6, 5, 1, 0]);
147+
expect(out[0].x).toEqual([-1, -2, 1, 1, -2, null, 0, 3]);
148+
expect(out[0].y).toEqual([2, 1, 1, 2, 3, 4, 1, 3]);
149+
expect(out[0].ids).toEqual(['n1', 'n0', 'p3', 'p1', 'n2', 'n3', 'z', 'p2']);
150+
expect(out[0].marker.color).toEqual([0.2, 0.1, 0.4, 0.2, 0.3, 0.4, 0.1, 0.3]);
151+
expect(out[0].marker.size).toEqual([20, 10, 10, 6, 5, 3, 1, 0]);
146152
expect(out[0].transforms[0]._indexToPoints).toEqual({
147153
0: [1],
148154
1: [0],
149-
2: [6],
155+
2: [7],
150156
3: [4],
151157
4: [2],
152-
5: [3],
153-
6: [5]
158+
5: [6],
159+
6: [3],
160+
7: [5],
154161
});
155162
});
156163

@@ -175,25 +182,25 @@ describe('Test sort transform calc:', function() {
175182

176183
var out = _transform([trace]);
177184

178-
expect(out[0].x).toEqual(['F', 'D', 'C', 'E', 'A', 'G', 'B']);
179-
expect(out[0].y).toEqual([3, 1, 3, 2, 1, 1, 2]);
180-
expect(out[0].ids).toEqual(['p2', 'z', 'n2', 'p1', 'n0', 'p3', 'n1']);
181-
expect(out[0].marker.size).toEqual([0, 1, 5, 6, 10, 10, 20]);
182-
expect(out[0].marker.color).toEqual([0.3, 0.1, 0.3, 0.2, 0.1, 0.4, 0.2]);
185+
expect(out[0].x).toEqual(['F', 'D', 'G', 'C', 'E', 'A', 'H', 'B']);
186+
expect(out[0].y).toEqual([3, 1, 4, 3, 2, 1, 1, 2]);
187+
expect(out[0].ids).toEqual(['p2', 'z', 'n3', 'n2', 'p1', 'n0', 'p3', 'n1']);
188+
expect(out[0].marker.size).toEqual([0, 1, 3, 5, 6, 10, 10, 20]);
189+
expect(out[0].marker.color).toEqual([0.3, 0.1, 0.4, 0.3, 0.2, 0.1, 0.4, 0.2]);
183190
});
184191

185192
it('should sort via custom targets', function() {
186193
var out = _transform([extend({
187194
transforms: [{
188-
target: [10, 20, 30, 10, 20, 30, 0]
195+
target: [10, 20, 30, 10, 20, 30, null, 0]
189196
}]
190197
})]);
191198

192-
expect(out[0].x).toEqual([1, -2, 0, -1, 1, -2, 3]);
193-
expect(out[0].y).toEqual([1, 1, 1, 2, 2, 3, 3]);
194-
expect(out[0].ids).toEqual(['p3', 'n0', 'z', 'n1', 'p1', 'n2', 'p2']);
195-
expect(out[0].marker.color).toEqual([0.4, 0.1, 0.1, 0.2, 0.2, 0.3, 0.3]);
196-
expect(out[0].marker.size).toEqual([10, 10, 1, 20, 6, 5, 0]);
199+
expect(out[0].x).toEqual([1, -2, 0, -1, 1, -2, 3, null]);
200+
expect(out[0].y).toEqual([1, 1, 1, 2, 2, 3, 3, 4]);
201+
expect(out[0].ids).toEqual(['p3', 'n0', 'z', 'n1', 'p1', 'n2', 'p2', 'n3']);
202+
expect(out[0].marker.color).toEqual([0.4, 0.1, 0.1, 0.2, 0.2, 0.3, 0.3, 0.4]);
203+
expect(out[0].marker.size).toEqual([10, 10, 1, 20, 6, 5, 0, 3]);
197204
});
198205

199206
it('should truncate transformed arrays to target array length (short target case)', function() {
@@ -237,19 +244,19 @@ describe('Test sort transform calc:', function() {
237244
transforms: [{ target: 'text' }]
238245
})]);
239246

240-
expect(out[0].x).toEqual([1, -2, 3, -1, 1, -2, 0]);
241-
expect(out[0].y).toEqual([1, 3, 3, 2, 2, 1, 1]);
242-
expect(out[0].ids).toEqual(['p3', 'n2', 'p2', 'n1', 'p1', 'n0', 'z']);
243-
expect(out[0].marker.color).toEqual([0.4, 0.3, 0.3, 0.2, 0.2, 0.1, 0.1]);
244-
expect(out[0].marker.size).toEqual([10, 5, 0, 20, 6, 10, 1]);
245-
expect(out[0]._length).toBe(7);
246-
247-
expect(out[1].x).toEqual([-2, -1, -2, 0, 1, 3, 1]);
248-
expect(out[1].y).toEqual([1, 2, 3, 1, 2, 3, 1]);
249-
expect(out[1].ids).toEqual(['n0', 'n1', 'n2', 'z', 'p1', 'p2', 'p3']);
250-
expect(out[1].marker.color).toEqual([0.1, 0.2, 0.3, 0.1, 0.2, 0.3, 0.4]);
251-
expect(out[1].marker.size).toEqual([10, 20, 5, 1, 6, 0, 10]);
252-
expect(out[1]._length).toBe(7);
247+
expect(out[0].x).toEqual([null, -2, 3, -1, 1, -2, 0, 1]);
248+
expect(out[0].y).toEqual([4, 3, 3, 2, 2, 1, 1, 1]);
249+
expect(out[0].ids).toEqual(['n3', 'n2', 'p2', 'n1', 'p1', 'n0', 'z', 'p3']);
250+
expect(out[0].marker.color).toEqual([0.4, 0.3, 0.3, 0.2, 0.2, 0.1, 0.1, 0.4]);
251+
expect(out[0].marker.size).toEqual([3, 5, 0, 20, 6, 10, 1, 10]);
252+
expect(out[0]._length).toBe(8);
253+
254+
expect(out[1].x).toEqual([-2, -1, -2, 0, 1, 3, null, 1]);
255+
expect(out[1].y).toEqual([1, 2, 3, 1, 2, 3, 4, 1]);
256+
expect(out[1].ids).toEqual(['n0', 'n1', 'n2', 'z', 'p1', 'p2', 'n3', 'p3']);
257+
expect(out[1].marker.color).toEqual([0.1, 0.2, 0.3, 0.1, 0.2, 0.3, 0.4, 0.4]);
258+
expect(out[1].marker.size).toEqual([10, 20, 5, 1, 6, 0, 3, 10]);
259+
expect(out[1]._length).toBe(8);
253260
});
254261
});
255262

0 commit comments

Comments
 (0)