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.

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

Implement strict autotypenumbers #5240

Merged
merged 25 commits into from
Nov 4, 2020
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
cbac3ae
issue 5195 - implement convertnumeric for axes and inherit from layout
archmoj Oct 29, 2020
c4224f4
adjust autoType return when convertNumeric is disabled - add tests
archmoj Oct 30, 2020
3b1dd0f
fixups and add polar tests
archmoj Oct 30, 2020
48fabdd
add gl3d tests
archmoj Oct 30, 2020
5aca807
add box plot tests an improve gl3d tests
archmoj Oct 30, 2020
84907dc
add tests for carpet
archmoj Oct 30, 2020
04da8a1
Update src/plots/cartesian/layout_attributes.js
archmoj Nov 2, 2020
01115cd
Update src/plots/layout_attributes.js
archmoj Nov 2, 2020
fb67c82
revise new attribute names and val types
archmoj Nov 3, 2020
f1c0e31
add early return for empty arrays to test category case and a bit of …
archmoj Nov 4, 2020
837761c
maintain detecting dates using strict mode
archmoj Nov 4, 2020
b73d395
pass convertNumeric to linearOK and category
archmoj Nov 4, 2020
0224fa0
store type of
archmoj Nov 4, 2020
d218bab
add early return for dates - remove unnecessary early return in linea…
archmoj Nov 4, 2020
b800a65
add early returns - array of arrays is not relevant to date, category…
archmoj Nov 4, 2020
f6d8c39
flat input array before passing to autoType functions
archmoj Nov 4, 2020
560405c
revert description change
archmoj Nov 4, 2020
4697f9c
flat 2d arrays before passing to autotypes
archmoj Nov 4, 2020
688438c
handle typed arrays inside a 2d array - plus a bit of optimization
archmoj Nov 4, 2020
e02deaf
add tests for autotype - case of 2d arrays
archmoj Nov 4, 2020
4b5bfcc
correct keys in description
archmoj Nov 4, 2020
08aa300
drop unused option in ternary
archmoj Nov 4, 2020
1198e56
add noMultiCategory option for polar
archmoj Nov 4, 2020
99a2015
apply layout.calnedar and layout.autotypenumbers in box defaults
archmoj Nov 4, 2020
6fdf559
fixup box plot test to have a layout object
archmoj Nov 4, 2020
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
4 changes: 3 additions & 1 deletion src/plots/cartesian/axes.js
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,9 @@ var getDataConversions = axes.getDataConversions = function(gd, trace, target, t
// setup the data-to-calc method.
if(Array.isArray(d2cTarget)) {
ax = {
type: autoType(targetArray),
type: autoType(targetArray, undefined, {
autotypenumbers: gd._fullLayout.autotypenumbers
}),
_categories: []
};
axes.setConvert(ax);
Expand Down
98 changes: 67 additions & 31 deletions src/plots/cartesian/axis_autotype.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,23 +14,49 @@ var isNumeric = require('fast-isnumeric');
var Lib = require('../../lib');
var BADNUM = require('../../constants/numerical').BADNUM;

var isArrayOrTypedArray = Lib.isArrayOrTypedArray;
var isDateTime = Lib.isDateTime;
var cleanNumber = Lib.cleanNumber;
var round = Math.round;

module.exports = function autoType(array, calendar, opts) {
opts = opts || {};
var a = array;

var noMultiCategory = opts.noMultiCategory;
if(isArrayOrTypedArray(a) && !a.length) return '-';
if(!noMultiCategory && multiCategory(a)) return 'multicategory';
if(noMultiCategory && Array.isArray(a[0])) { // no need to flat typed arrays here
var b = [];
for(var i = 0; i < a.length; i++) {
if(isArrayOrTypedArray(a[i])) {
for(var j = 0; j < a[i].length; j++) {
b.push(a[i][j]);
}
}
}
a = b;
alexcjohnson marked this conversation as resolved.
Show resolved Hide resolved
}

if(moreDates(a, calendar)) return 'date';

if(!opts.noMultiCategory && multiCategory(array)) return 'multicategory';
if(moreDates(array, calendar)) return 'date';
if(category(array)) return 'category';
if(linearOK(array)) return 'linear';
else return '-';
var convertNumeric = opts.autotypenumbers !== 'strict'; // compare against strict, just in case autotypenumbers was not provided in opts
if(category(a, convertNumeric)) return 'category';
if(linearOK(a, convertNumeric)) return 'linear';

return '-';
};

function hasTypeNumber(v, convertNumeric) {
return convertNumeric ? isNumeric(v) : typeof v === 'number';
}

// is there at least one number in array? If not, we should leave
// ax.type empty so it can be autoset later
function linearOK(array) {
if(!array) return false;
function linearOK(a, convertNumeric) {
var len = a.length;

for(var i = 0; i < array.length; i++) {
if(isNumeric(array[i])) return true;
for(var i = 0; i < len; i++) {
if(hasTypeNumber(a[i], convertNumeric)) return true;
}

return false;
Expand All @@ -43,51 +69,61 @@ function linearOK(array) {
// numbers and a few dates
// as with categories, consider DISTINCT values only.
function moreDates(a, calendar) {
// test at most 1000 points, evenly spaced
var inc = Math.max(1, (a.length - 1) / 1000);
var dcnt = 0;
var ncnt = 0;
var len = a.length;

var inc = getIncrement(len);
var dats = 0;
var nums = 0;
var seen = {};

for(var i = 0; i < a.length; i += inc) {
var ai = a[Math.round(i)];
for(var f = 0; f < len; f += inc) {
var i = round(f);
var ai = a[i];
var stri = String(ai);
if(seen[stri]) continue;
seen[stri] = 1;

if(Lib.isDateTime(ai, calendar)) dcnt += 1;
if(isNumeric(ai)) ncnt += 1;
if(isDateTime(ai, calendar)) dats++;
if(isNumeric(ai)) nums++;
}

return (dcnt > ncnt * 2);
return dats > nums * 2;
}

// return increment to test at most 1000 points, evenly spaced
function getIncrement(len) {
return Math.max(1, (len - 1) / 1000);
}

// are the (x,y)-values in gd.data mostly text?
// require twice as many DISTINCT categories as distinct numbers
function category(a) {
// test at most 1000 points
var inc = Math.max(1, (a.length - 1) / 1000);
var curvenums = 0;
var curvecats = 0;
function category(a, convertNumeric) {
var len = a.length;

var inc = getIncrement(len);
var nums = 0;
var cats = 0;
var seen = {};

for(var i = 0; i < a.length; i += inc) {
var ai = a[Math.round(i)];
for(var f = 0; f < len; f += inc) {
var i = round(f);
var ai = a[i];
var stri = String(ai);
if(seen[stri]) continue;
seen[stri] = 1;

if(typeof ai === 'boolean') curvecats++;
else if(Lib.cleanNumber(ai) !== BADNUM) curvenums++;
else if(typeof ai === 'string') curvecats++;
var t = typeof ai;
if(t === 'boolean') cats++;
else if(convertNumeric ? cleanNumber(ai) !== BADNUM : t === 'number') nums++;
else if(t === 'string') cats++;
}

return curvecats > curvenums * 2;
return cats > nums * 2;
}

// very-loose requirements for multicategory,
// trace modules that should never auto-type to multicategory
// should be declared with 'noMultiCategory'
function multiCategory(a) {
return Lib.isArrayOrTypedArray(a[0]) && Lib.isArrayOrTypedArray(a[1]);
return isArrayOrTypedArray(a[0]) && isArrayOrTypedArray(a[1]);
}
13 changes: 13 additions & 0 deletions src/plots/cartesian/layout_attributes.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,19 @@ module.exports = {
'the axis in question.'
].join(' ')
},
autotypenumbers: {
valType: 'enumerated',
values: ['convert types', 'strict'],
dflt: 'convert types',
role: 'info',
editType: 'calc',
description: [
'Using *strict* a numeric string in trace data is not converted to a number.',
'Using *accept strings* a numeric string in trace data may be',
'treated as a number during automatic axis `type` detection.',
'Defaults to layout.autotypenumbers.'
].join(' ')
},
autorange: {
valType: 'enumerated',
values: [true, false, 'reversed'],
Expand Down
4 changes: 4 additions & 0 deletions src/plots/cartesian/layout_defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ function appendList(cont, k, item) {
}

module.exports = function supplyLayoutDefaults(layoutIn, layoutOut, fullData) {
var autotypenumbersDflt = layoutOut.autotypenumbers;

var ax2traces = {};
var xaMayHide = {};
var yaMayHide = {};
Expand Down Expand Up @@ -246,6 +248,7 @@ module.exports = function supplyLayoutDefaults(layoutIn, layoutOut, fullData) {
automargin: true,
visibleDflt: visibleDflt,
reverseDflt: reverseDflt,
autotypenumbersDflt: autotypenumbersDflt,
splomStash: ((layoutOut._splomAxes || {})[axLetter] || {})[axId]
};

Expand Down Expand Up @@ -310,6 +313,7 @@ module.exports = function supplyLayoutDefaults(layoutIn, layoutOut, fullData) {
automargin: true,
visibleDflt: false,
reverseDflt: false,
autotypenumbersDflt: autotypenumbersDflt,
splomStash: ((layoutOut._splomAxes || {})[axLetter] || {})[axId]
};

Expand Down
3 changes: 3 additions & 0 deletions src/plots/cartesian/type_defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ var autoType = require('./axis_autotype');
* name: axis object name (ie 'xaxis') if one should be stored
*/
module.exports = function handleTypeDefaults(containerIn, containerOut, coerce, options) {
coerce('autotypenumbers', options.autotypenumbersDflt);
var axType = coerce('type', (options.splomStash || {}).type);

if(axType === '-') {
Expand Down Expand Up @@ -68,6 +69,8 @@ function setAutoType(ax, data) {
opts.noMultiCategory = true;
}

opts.autotypenumbers = ax.autotypenumbers;

// check all boxes on this x axis to see
// if they're dates, numbers, or categories
if(isBoxWithoutPositionCoords(d0, axLetter)) {
Expand Down
1 change: 1 addition & 0 deletions src/plots/gl3d/layout/axis_attributes.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ module.exports = overrideAll({
type: extendFlat({}, axesAttrs.type, {
values: ['-', 'linear', 'log', 'date', 'category']
}),
autotypenumbers: axesAttrs.autotypenumbers,
autorange: axesAttrs.autorange,
rangemode: axesAttrs.rangemode,
range: extendFlat({}, axesAttrs.range, {
Expand Down
2 changes: 2 additions & 0 deletions src/plots/gl3d/layout/defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ module.exports = function supplyLayoutDefaults(layoutIn, layoutOut, fullData) {
font: layoutOut.font,
fullData: fullData,
getDfltFromLayout: getDfltFromLayout,
autotypenumbersDflt: layoutOut.autotypenumbers,
paper_bgcolor: layoutOut.paper_bgcolor,
calendar: layoutOut.calendar
});
Expand Down Expand Up @@ -109,6 +110,7 @@ function handleGl3dDefaults(sceneLayoutIn, sceneLayoutOut, coerce, opts) {
data: fullGl3dData,
bgColor: bgColorCombined,
calendar: opts.calendar,
autotypenumbersDflt: opts.autotypenumbersDflt,
fullLayout: opts.fullLayout
});

Expand Down
13 changes: 13 additions & 0 deletions src/plots/layout_attributes.js
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,19 @@ module.exports = {
'Sets the background color of the plotting area in-between x and y axes.'
].join(' ')
},
autotypenumbers: {
valType: 'enumerated',
values: ['convert types', 'strict'],
dflt: 'convert types',
role: 'info',
editType: 'calc',
description: [
'Using *strict* a numeric string in trace data is not converted to a number.',
'Using *accept strings* a numeric string in trace data may be',
alexcjohnson marked this conversation as resolved.
Show resolved Hide resolved
'treated as a number during automatic axis `type` detection.',
'This is the default value; however it could be overridden for individual axes.'
].join(' ')
},
separators: {
valType: 'string',
role: 'style',
Expand Down
2 changes: 2 additions & 0 deletions src/plots/plots.js
Original file line number Diff line number Diff line change
Expand Up @@ -1476,6 +1476,8 @@ plots.supplyLayoutGlobalDefaults = function(layoutIn, layoutOut, formatObj) {
layoutOut._dataTemplate = template.data;
}

coerce('autotypenumbers');

var globalFont = Lib.coerceFont(coerce, 'font');

coerce('title.text', layoutOut._dfltTitle.plot);
Expand Down
2 changes: 2 additions & 0 deletions src/plots/polar/layout_attributes.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ var radialAxisAttrs = {
type: extendFlat({}, axesAttrs.type, {
values: ['-', 'linear', 'log', 'date', 'category']
}),
autotypenumbers: axesAttrs.autotypenumbers,

autorange: extendFlat({}, axesAttrs.autorange, {editType: 'plot'}),
rangemode: {
Expand Down Expand Up @@ -179,6 +180,7 @@ var angularAxisAttrs = {
'If *category, use `period` to set the number of integer coordinates around polar axis.'
].join(' ')
},
autotypenumbers: axesAttrs.autotypenumbers,

categoryorder: axesAttrs.categoryorder,
categoryarray: axesAttrs.categoryarray,
Expand Down
10 changes: 7 additions & 3 deletions src/plots/polar/layout_defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ function handleDefaults(contIn, contOut, coerce, opts) {
axOut._traceIndices = subplotData.map(function(t) { return t._expandedIndex; });

var dataAttr = constants.axisName2dataArray[axName];
var axType = handleAxisTypeDefaults(axIn, axOut, coerceAxis, subplotData, dataAttr);
var axType = handleAxisTypeDefaults(axIn, axOut, coerceAxis, subplotData, dataAttr, opts);

handleCategoryOrderDefaults(axIn, axOut, coerceAxis, {
axData: subplotData,
Expand Down Expand Up @@ -187,7 +187,8 @@ function handleDefaults(contIn, contOut, coerce, opts) {
}
}

function handleAxisTypeDefaults(axIn, axOut, coerce, subplotData, dataAttr) {
function handleAxisTypeDefaults(axIn, axOut, coerce, subplotData, dataAttr, options) {
var autotypenumbers = coerce('autotypenumbers', options.autotypenumbersDflt);
var axType = coerce('type');

if(axType === '-') {
Expand All @@ -201,7 +202,9 @@ function handleAxisTypeDefaults(axIn, axOut, coerce, subplotData, dataAttr) {
}

if(trace && trace[dataAttr]) {
axOut.type = autoType(trace[dataAttr], 'gregorian');
axOut.type = autoType(trace[dataAttr], 'gregorian', {
autotypenumbers: autotypenumbers
alexcjohnson marked this conversation as resolved.
Show resolved Hide resolved
});
}

if(axOut.type === '-') {
Expand All @@ -224,6 +227,7 @@ module.exports = function supplyLayoutDefaults(layoutIn, layoutOut, fullData) {
attributes: layoutAttributes,
handleDefaults: handleDefaults,
font: layoutOut.font,
autotypenumbersDflt: layoutOut.autotypenumbers,
paper_bgcolor: layoutOut.paper_bgcolor,
fullData: fullData,
layoutOut: layoutOut
Expand Down
1 change: 1 addition & 0 deletions src/plots/ternary/layout_defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ module.exports = function supplyLayoutDefaults(layoutIn, layoutOut, fullData) {
attributes: layoutAttributes,
handleDefaults: handleTernaryDefaults,
font: layoutOut.font,
autotypenumbersDflt: layoutOut.autotypenumbers,
alexcjohnson marked this conversation as resolved.
Show resolved Hide resolved
paper_bgcolor: layoutOut.paper_bgcolor
});
};
Expand Down
9 changes: 7 additions & 2 deletions src/traces/box/defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,11 @@ function handleSampleDefaults(traceIn, traceOut, coerce, layout) {
var yLen = yDims && Lib.minRowLength(y);
var xLen = xDims && Lib.minRowLength(x);

var calendar = 'gregorian'; // TODO: should we use ax.calendar here?
var opts = {
autotypenumbers: 'convert types' // TODO: should we use ax.autotypenumbers here?
alexcjohnson marked this conversation as resolved.
Show resolved Hide resolved
alexcjohnson marked this conversation as resolved.
Show resolved Hide resolved
};

var defaultOrientation, len;
if(traceOut._hasPreCompStats) {
switch(String(xDims) + String(yDims)) {
Expand Down Expand Up @@ -160,7 +165,7 @@ function handleSampleDefaults(traceIn, traceOut, coerce, layout) {
var hasCategories = false;
var i;
for(i = 0; i < x.length; i++) {
if(autoType(x[i]) === 'category') {
if(autoType(x[i], calendar, opts) === 'category') {
hasCategories = true;
break;
}
Expand All @@ -171,7 +176,7 @@ function handleSampleDefaults(traceIn, traceOut, coerce, layout) {
len = Math.min(sLen, xLen, y.length);
} else {
for(i = 0; i < y.length; i++) {
if(autoType(y[i]) === 'category') {
if(autoType(y[i], calendar, opts) === 'category') {
hasCategories = true;
break;
}
Expand Down
1 change: 1 addition & 0 deletions src/traces/carpet/ab_defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ function mimickAxisDefaults(traceIn, traceOut, fullLayout, dfltColor) {
calendar: traceOut.calendar,
dfltColor: dfltColor,
bgColor: fullLayout.paper_bgcolor,
autotypenumbersDflt: fullLayout.autotypenumbers,
fullLayout: fullLayout
};

Expand Down
1 change: 1 addition & 0 deletions src/traces/carpet/axis_attributes.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ module.exports = {
'the axis in question.'
].join(' ')
},
autotypenumbers: axesAttrs.autotypenumbers,
autorange: {
valType: 'enumerated',
values: [true, false, 'reversed'],
Expand Down
5 changes: 4 additions & 1 deletion src/traces/carpet/axis_defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ module.exports = function handleAxisDefaults(containerIn, containerOut, options)
}

// now figure out type and do some more initialization
coerce('autotypenumbers', options.autotypenumbersDflt);
var axType = coerce('type');
if(axType === '-') {
if(options.data) setAutoType(containerOut, options.data);
Expand Down Expand Up @@ -219,5 +220,7 @@ function setAutoType(ax, data) {
var calAttr = axLetter + 'calendar';
var calendar = ax[calAttr];

ax.type = autoType(data, calendar);
ax.type = autoType(data, calendar, {
autotypenumbers: ax.autotypenumbers
});
}
Loading