Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cartesian 2dMap: implement sorting of categorical axes #3827

Merged
merged 21 commits into from
May 8, 2019
Merged

cartesian 2dMap: implement sorting of categorical axes #3827

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
e1716c7
heatmap: reorder data based on categoryarray
antoinerg Apr 30, 2019
d8ef1d7
heatmap: :lock: down categoryorder and categoryarray with tests
antoinerg May 1, 2019
9735683
heatmap: handle undefined axes in clean_2d_array
antoinerg May 1, 2019
5798dd8
heatmap: properly sort categorical data when z data is a 1D array
antoinerg May 1, 2019
56c6a4d
heatmap: do not sort categorical axes for contour
antoinerg May 1, 2019
f878387
heatmap: only sort categorical axes for trace type heatmap
antoinerg May 2, 2019
a34ca45
heatmap: fix - check trace exists before checking type
antoinerg May 2, 2019
6b06516
heatmap: handle categorical axis sorting except for (contour)carpet
antoinerg May 2, 2019
59edb57
heatmap calc: improve style, test contour and heatmap
antoinerg May 3, 2019
22cfa82
:lock: down category ordering in histogram2d(contour) with tests
antoinerg May 5, 2019
02e4089
:lock: down category ordering in heatmapgl with tests
antoinerg May 6, 2019
5160948
test: tag @flaky test, :hocho: useless lines
antoinerg May 6, 2019
e966030
categorical heatmap: handle missing categories, 1D data and hover
antoinerg May 6, 2019
ae3ecf5
categorical contour: fix jasmine test
antoinerg May 6, 2019
e68c170
categorical 2dMap: fill missing category in input data with undefined
antoinerg May 7, 2019
ebf545f
categorial 2dMap: replace undefined with BADNUM
antoinerg May 7, 2019
793c2a3
categorical 2dMap: update mock and baseline
antoinerg May 7, 2019
4d4b0b7
categorical 2dmap: fix flaky tests
antoinerg May 7, 2019
f180c79
categorical 2dmap: do not use indexOf to find trace-level category ma…
antoinerg May 7, 2019
0c41776
categorical cartesian: convert numbers to strings for _categories
antoinerg May 7, 2019
c7cd1f3
categorical 2dMap: remove hasOwnProperty() call
antoinerg May 7, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/traces/heatmap/calc.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,14 +64,17 @@ module.exports = function calc(gd, trace) {
} else {
x = trace.x ? xa.makeCalcdata(trace, 'x') : [];
y = trace.y ? ya.makeCalcdata(trace, 'y') : [];

trace._x = x;
trace._y = y;
etpinard marked this conversation as resolved.
Show resolved Hide resolved
}

x0 = trace.x0;
dx = trace.dx;
y0 = trace.y0;
dy = trace.dy;

z = clean2dArray(zIn, trace.transpose);
z = clean2dArray(zIn, trace.transpose, trace, xa, ya);

if(isContour || trace.connectgaps) {
trace._emptypoints = findEmpties(z);
Expand Down
15 changes: 13 additions & 2 deletions src/traces/heatmap/clean_2d_array.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

var isNumeric = require('fast-isnumeric');

module.exports = function clean2dArray(zOld, transpose) {
module.exports = function clean2dArray(zOld, transpose, trace, xa, ya) {
etpinard marked this conversation as resolved.
Show resolved Hide resolved
var rowlen, collen, getCollen, old2new, i, j;

function cleanZvalue(v) {
Expand All @@ -30,12 +30,23 @@ module.exports = function clean2dArray(zOld, transpose) {
old2new = function(zOld, i, j) { return zOld[i][j]; };
}

var xMap = function(i) {return i;};
etpinard marked this conversation as resolved.
Show resolved Hide resolved
var yMap = function(i) {return i;};
if(trace && trace.type === 'heatmap') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. Why just heatmap?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was the low-hanging fruit. I guess I should enlarge the scope of this PR and implement sorting of categorical axes for contour, histogram2d and all other traces relying on heatmap's calc routine.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes please!

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

definitely useful for histogram2d! I have qualms from a viz-theory standpoint about sorting an axis of a contour plot: if you're drawing contours, which interpolate between data points, it strongly implies a continuous axis, so you really have no business with categories unless they're some sort of a hack of what's really a continuous axis... and sorting a continuous axis is meaningless.

That said, we allow it in the schema, therefore it's a bug if it doesn't work!

Copy link
Contributor Author

@antoinerg antoinerg May 2, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Histogram2d and contour are now working in 6b06516

By allowing all traces (except carpet and contourcarpet), the only test that would fail was in bar_test.js. In it, we would create a contour trace without x and y data. In that case, trace._x and trace._y would be empty and it failed. To fix this, we check whether those arrays are empty and ignore categorical sorting if they are. After all, if the data has no categories attached to it, it's not clear how it should be sorted.

if(ya && ya.type === 'category') {
yMap = function(i) {return trace._y[i];};
}
etpinard marked this conversation as resolved.
Show resolved Hide resolved
if(xa && xa.type === 'category') {
xMap = function(i) {return trace._x[i];};
}
}

var zNew = new Array(rowlen);

for(i = 0; i < rowlen; i++) {
collen = getCollen(zOld, i);
zNew[i] = new Array(collen);
for(j = 0; j < collen; j++) zNew[i][j] = cleanZvalue(old2new(zOld, i, j));
for(j = 0; j < collen; j++) zNew[i][j] = cleanZvalue(old2new(zOld, yMap(i), xMap(j)));
}

return zNew;
Expand Down
Binary file added test/image/baselines/heatmap_categoryorder.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
26 changes: 26 additions & 0 deletions test/image/mocks/heatmap_categoryorder.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
{
"layout": {
"xaxis": {
"type": "category",
"categoryorder": "category descending"
},
"yaxis": {
"type": "category",
"categoryorder": "category descending"
},
"height": 400,
"width": 400
},
"data": [{
"type": "heatmap",

"x": [3, 2, 1, 0],
"y": ["d", "c", "b", "a"],
"z": [
[100, 75, 50, 0],
[90, 65, 40, 0],
[80, 55, 30, 0],
[0, 0, 0, 0]
]
}]
}
45 changes: 45 additions & 0 deletions test/jasmine/tests/heatmap_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -491,6 +491,51 @@ describe('heatmap calc', function() {
expect(out.y).toBeCloseToArray([0, 4, 8]);
expect(out.z).toBeCloseTo2DArray([[1, 2, 3], [3, 1, 2]]);
});

it('should handle axis categoryorder', function() {
var mock = require('@mocks/heatmap_categoryorder');
var data = mock.data[0];
var layout = mock.layout;

// sort x axis categories
var mockLayout = Lib.extendDeep({}, layout);
var out = _calc(data, mockLayout);
mockLayout.xaxis.categoryorder = 'category ascending';
var out1 = _calc(data, mockLayout);

expect(out._xcategories).toEqual(out1._xcategories.reverse());
// Check z data is also sorted
for(var i = 0; i < out.z.length; i++) {
expect(out1.z[i]).toEqual(out.z[i].reverse());
}

// sort y axis categories
mockLayout = Lib.extendDeep({}, layout);
out = _calc(data, mockLayout);
mockLayout.yaxis.categoryorder = 'category ascending';
out1 = _calc(data, mockLayout);

expect(out._ycategories).toEqual(out1._ycategories.reverse());
// Check z data is also sorted
expect(out1.z).toEqual(out.z.reverse());
});

it('should handle axis categoryarray', function() {
var mock = require('@mocks/heatmap_categoryorder');
var data = mock.data[0];
var layout = mock.layout;

layout.xaxis.categoryorder = 'array';
layout.xaxis.categoryarray = [2, 3, 0, 1];
layout.yaxis.categoryorder = 'array';
layout.yaxis.categoryarray = ['a', 'd', 'b', 'c'];

var out = _calc(data, layout);

expect(out._xcategories).toEqual(layout.xaxis.categoryarray, 'xaxis should reorder');
expect(out._ycategories).toEqual(layout.yaxis.categoryarray, 'yaxis should reorder');
expect(out.z[0][0]).toEqual(65);
});
});

describe('heatmap plot', function() {
Expand Down