Skip to content

Commit

Permalink
fixes #1460 - refactor initial category ordering
Browse files Browse the repository at this point in the history
- 🔪 'pure' ordered_categories.js and move logic to
  category_order_defaults (no need for two files for this stuff here)
- replace d3.bisector-based search-and-replace algo by lookup object,
  which bring number categories on par with their string rep
  (e.g. 1 same as '1' - see issue #1460)
- use native array sort() to order categories instead of d3 version
  • Loading branch information
etpinard committed Mar 30, 2018
1 parent 463d7ce commit 80c13a9
Show file tree
Hide file tree
Showing 6 changed files with 105 additions and 95 deletions.
7 changes: 1 addition & 6 deletions src/plots/cartesian/axis_defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ var handleTickLabelDefaults = require('./tick_label_defaults');
var handleCategoryOrderDefaults = require('./category_order_defaults');
var handleLineGridDefaults = require('./line_grid_defaults');
var setConvert = require('./set_convert');
var orderedCategories = require('./ordered_categories');

/**
* options: object containing:
Expand Down Expand Up @@ -60,11 +59,7 @@ module.exports = function handleAxisDefaults(containerIn, containerOut, coerce,
coerce('range');
containerOut.cleanRange();

handleCategoryOrderDefaults(containerIn, containerOut, coerce);
containerOut._initialCategories = axType === 'category' ?
orderedCategories(letter, containerOut.categoryorder, containerOut.categoryarray, options.data) :
[];

handleCategoryOrderDefaults(containerIn, containerOut, coerce, options);

if(axType !== 'category' && !options.noHover) coerce('hoverformat');

Expand Down
72 changes: 66 additions & 6 deletions src/plots/cartesian/category_order_defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,25 +8,85 @@

'use strict';

function findCategories(ax, opts) {
var dataAttr = opts.dataAttr || ax._id.charAt(0);
var lookup = {};
var axData;
var i, j;

module.exports = function handleCategoryOrderDefaults(containerIn, containerOut, coerce) {
if(containerOut.type !== 'category') return;
if(opts.axData) {
// non-x/y case
axData = opts.axData;
} else {
// x/y case
axData = [];
for(i = 0; i < opts.data.length; i++) {
var trace = opts.data[i];
if(trace[dataAttr + 'axis'] === ax._id) {
axData.push(trace);
}
}
}

for(i = 0; i < axData.length; i++) {
var vals = axData[i][dataAttr];
for(j = 0; j < vals.length; j++) {
var v = vals[j];
if(v !== null && v !== undefined) {
lookup[v] = 1;
}
}
}

return Object.keys(lookup);
}

var arrayIn = containerIn.categoryarray,
orderDefault;
/**
* Fills in category* default and initial categories.
*
* @param {object} containerIn : input axis object
* @param {object} containerOut : full axis object
* @param {function} coerce : Lib.coerce fn wrapper
* @param {object} opts :
* - data {array} : (full) data trace
* OR
* - axData {array} : (full) data associated with axis being coerced here
* - dataAttr {string} : attribute name corresponding to coordinate array
*/
module.exports = function handleCategoryOrderDefaults(containerIn, containerOut, coerce, opts) {
if(containerOut.type !== 'category') return;

var arrayIn = containerIn.categoryarray;
var isValidArray = (Array.isArray(arrayIn) && arrayIn.length > 0);

// override default 'categoryorder' value when non-empty array is supplied
var orderDefault;
if(isValidArray) orderDefault = 'array';

var order = coerce('categoryorder', orderDefault);
var array;

// coerce 'categoryarray' only in array order case
if(order === 'array') coerce('categoryarray');
if(order === 'array') {
array = coerce('categoryarray');
}

// cannot set 'categoryorder' to 'array' with an invalid 'categoryarray'
if(!isValidArray && order === 'array') {
containerOut.categoryorder = 'trace';
order = containerOut.categoryorder = 'trace';
}

// set up things for makeCalcdata
if(order === 'trace') {
containerOut._initialCategories = [];
} else if(order === 'array') {
containerOut._initialCategories = array.slice();
} else {
array = findCategories(containerOut, opts).sort();
if(order === 'category ascending') {
containerOut._initialCategories = array;
} else if(order === 'category descending') {
containerOut._initialCategories = array.reverse();
}
}
};
70 changes: 0 additions & 70 deletions src/plots/cartesian/ordered_categories.js

This file was deleted.

9 changes: 4 additions & 5 deletions src/plots/polar/layout_defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ var handleTickLabelDefaults = require('../cartesian/tick_label_defaults');
var handleCategoryOrderDefaults = require('../cartesian/category_order_defaults');
var handleLineGridDefaults = require('../cartesian/line_grid_defaults');
var autoType = require('../cartesian/axis_autotype');
var orderedCategories = require('../cartesian/ordered_categories');
var setConvert = require('../cartesian/set_convert');

var setConvertAngular = require('./helpers').setConvertAngular;
Expand Down Expand Up @@ -56,10 +55,10 @@ function handleDefaults(contIn, contOut, coerce, opts) {
var dataAttr = constants.axisName2dataArray[axName];
var axType = handleAxisTypeDefaults(axIn, axOut, coerceAxis, subplotData, dataAttr);

handleCategoryOrderDefaults(axIn, axOut, coerceAxis);
axOut._initialCategories = axType === 'category' ?
orderedCategories(dataAttr, axOut.categoryorder, axOut.categoryarray, subplotData) :
[];
handleCategoryOrderDefaults(axIn, axOut, coerceAxis, {
axData: subplotData,
dataAttr: dataAttr
});

var visible = coerceAxis('visible');
setConvert(axOut, layoutOut);
Expand Down
13 changes: 5 additions & 8 deletions src/traces/carpet/axis_defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,12 @@ var handleTickValueDefaults = require('../../plots/cartesian/tick_value_defaults
var handleTickLabelDefaults = require('../../plots/cartesian/tick_label_defaults');
var handleCategoryOrderDefaults = require('../../plots/cartesian/category_order_defaults');
var setConvert = require('../../plots/cartesian/set_convert');
var orderedCategories = require('../../plots/cartesian/ordered_categories');
var autoType = require('../../plots/cartesian/axis_autotype');

/**
* options: object containing:
*
* letter: 'x' or 'y'
* letter: 'a' or 'b'
* title: name of the axis (ie 'Colorbar') to go in default title
* name: axis object name (ie 'xaxis') if one should be stored
* font: the default font to inherit
Expand Down Expand Up @@ -133,7 +132,10 @@ module.exports = function handleAxisDefaults(containerIn, containerOut, options)

handleTickValueDefaults(containerIn, containerOut, coerce, axType);
handleTickLabelDefaults(containerIn, containerOut, coerce, axType, options);
handleCategoryOrderDefaults(containerIn, containerOut, coerce);
handleCategoryOrderDefaults(containerIn, containerOut, coerce, {
data: options.data,
dataAttr: letter
});

var gridColor = coerce2('gridcolor', addOpacity(dfltColor, 0.3));
var gridWidth = coerce2('gridwidth');
Expand Down Expand Up @@ -176,11 +178,6 @@ module.exports = function handleAxisDefaults(containerIn, containerOut, options)
}
}

// fill in categories
containerOut._initialCategories = axType === 'category' ?
orderedCategories(letter, containerOut.categoryorder, containerOut.categoryarray, options.data) :
[];

if(containerOut.showticklabels === 'none') {
delete containerOut.tickfont;
delete containerOut.tickangle;
Expand Down
29 changes: 29 additions & 0 deletions test/jasmine/tests/calcdata_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -883,6 +883,35 @@ describe('calculated data and points', function() {
expect(gd.calcdata[2][2]).toEqual(jasmine.objectContaining({x: 0, y: 11 + 20 + 32}));
});
});

it('should order categories per axis', function() {
Plotly.plot(gd, [
{x: ['a', 'c', 'g', 'e']},
{x: ['b', 'h', 'f', 'd'], xaxis: 'x2'}
], {
xaxis: {categoryorder: 'category ascending', domain: [0, 0.4]},
xaxis2: {categoryorder: 'category descending', domain: [0.6, 1]}
});

expect(gd._fullLayout.xaxis._categories).toEqual(['a', 'c', 'e', 'g']);
expect(gd._fullLayout.xaxis2._categories).toEqual(['h', 'f', 'd', 'b']);
});

it('should consider number categories and their string representation to be the same', function() {
Plotly.plot(gd, [{
x: ['a', 'b', 1, '1'],
y: [1, 2, 3, 4]
}], {
xaxis: {type: 'category'}
});

expect(gd._fullLayout.xaxis._categories).toEqual(['a', 'b', 1]);
expect(gd._fullLayout.xaxis._categoriesMap).toEqual({
'1': 2,
'a': 0,
'b': 1
});
});
});

describe('customdata', function() {
Expand Down

0 comments on commit 80c13a9

Please sign in to comment.