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

Period positioning for Cartesian traces #5074

Merged
merged 35 commits into from
Sep 29, 2020
Merged
Show file tree
Hide file tree
Changes from 30 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
95d253d
introduce period positioning for cartesian traces
archmoj Aug 13, 2020
dd05f3c
add image tests for period positioning on cartesian traces
archmoj Aug 13, 2020
a4e6bba
handle monthly periods
archmoj Sep 1, 2020
7341abe
Merge remote-tracking branch 'origin/master' into period-positioning
archmoj Sep 2, 2020
bdffa60
update period labels on baselines
archmoj Sep 2, 2020
8de4021
update gl2d baseline
archmoj Sep 3, 2020
103e710
compute the exact number of days for mothly periods
archmoj Sep 2, 2020
6321beb
support positive integers for months
archmoj Sep 3, 2020
35b7954
ensure positive ms periods
archmoj Sep 3, 2020
42d4d8d
fixup monthly mock
archmoj Sep 3, 2020
33cf742
fixup first month and improve test
archmoj Sep 3, 2020
0212372
improve image test by using relavant tickformats and colors
archmoj Sep 3, 2020
1948e03
Merge remote-tracking branch 'origin/master' into period-positioning
archmoj Sep 8, 2020
14c5512
improve monthly and yearly periods and implement (x|y)period0
archmoj Sep 12, 2020
46ec3f4
calculate years from months - add 2020 Mondays test
archmoj Sep 14, 2020
f1f0995
add tick0 to the mock
archmoj Sep 14, 2020
866b069
move align period step to traces so that we could record original pos…
archmoj Sep 16, 2020
8d9411b
display original position in hover for scatter and bar traces
archmoj Sep 16, 2020
1952483
initial attempt to use Lib.incrementMonth to compute endDate
archmoj Sep 17, 2020
6b68e34
rename variable name
archmoj Sep 17, 2020
be52281
remove time zone offset and use UTC getters
archmoj Sep 17, 2020
afa8965
separate monthly periods from daily periods defined by milliseconds
archmoj Sep 17, 2020
05bb135
fix period0 leap year in mock
archmoj Sep 17, 2020
94a164e
display start and end periods on hover
archmoj Sep 17, 2020
ba48115
fixup histogram & histogram2d hover
archmoj Sep 18, 2020
74070e0
refactor instanceOrPeriod function
archmoj Sep 21, 2020
f0c6d80
add jasmine tests for hover on period positions
archmoj Sep 21, 2020
a90f95d
Do not display start and end periods on hover by default
archmoj Sep 22, 2020
69d6af9
fix hover for funnel, waterfall, heatmap, contour, box, ohlc, candles…
archmoj Sep 22, 2020
222b5fe
improve scattergl hover and tests
archmoj Sep 23, 2020
5ae9e1e
Merge branch 'master' into period-positioning
archmoj Sep 25, 2020
83b22a0
apply AlexJ's suggestion to use Lib.incrementMonth and simplify case …
archmoj Sep 25, 2020
397bf63
set period0 to be on first Monday of 2000
archmoj Sep 28, 2020
083eae3
use a Sunday instead of Monday for weekly period0
archmoj Sep 28, 2020
72f6de8
reuse Lib.dateTick0
archmoj Sep 28, 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
92 changes: 92 additions & 0 deletions src/plots/cartesian/align_period.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
/**
* Copyright 2012-2020, Plotly, Inc.
* All rights reserved.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

'use strict';

var isNumeric = require('fast-isnumeric');
var Lib = require('../../lib');
var ms2DateTime = Lib.ms2DateTime;
var dateTime2ms = Lib.dateTime2ms;
var incrementMonth = Lib.incrementMonth;
var ONEDAY = require('../../constants/numerical').ONEDAY;

module.exports = function alignPeriod(trace, ax, axLetter, vals) {
if(ax.type !== 'date') return vals;

var alignment = trace[axLetter + 'periodalignment'];
if(!alignment) return vals;

var period = trace[axLetter + 'period'];
var mPeriod, dPeriod;
if(isNumeric(period)) {
dPeriod = +period;
dPeriod /= ONEDAY; // convert milliseconds to days
dPeriod = Math.round(dPeriod);
if(dPeriod <= 0) return vals;
} else if(typeof period === 'string' && period.charAt(0) === 'M') {
var n = +(period.substring(1));
if(n > 0 && Math.round(n) === n) {
mPeriod = n;
} else return vals;
}

var calendar = ax.calendar;

var isStart = 'start' === alignment;
// var isMiddle = 'middle' === alignment;
var isEnd = 'end' === alignment;

var period0 = trace[axLetter + 'period0'];
var base = dateTime2ms(period0, calendar) || 0;

var newVals = [];
var len = vals.length;
for(var i = 0; i < len; i++) {
var v = vals[i] - base;

var dateStr = ms2DateTime(v, 0, calendar);
var d = new Date(dateStr);
var year = d.getUTCFullYear();
var month = d.getUTCMonth();
var day = d.getUTCDate();
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this approach is still going to have problems with time zones - specifically, I think the day will be wrong this way if you're in the eastern hemisphere.

There would be ways around this, but we've already solved these problems and all sorts of other edge cases with functions in lib/dates so you never directly need Date objects again and all your processing is with millisecond numbers. What I was thinking of in #5074 (comment) was something like:

var v = vals[i];
// guess at how many periods away from base we are
var nEstimated = Math.round((v - base) / (mPeriod * ONEAVGMONTH));
var endTime = incrementMonth(base, mPeriod * nEstimated, calendar);

// iterate to get the exact bounds before and after v
// there may be ways to make this faster, but most of the time
// you'll only execute each loop zero or one time.
while (endTime > v) {
    endTime = incrementMonth(endTime, -mPeriod, calendar);
}
while (endTime <= v) {
    endTime = incrementMonth(endTime, mPeriod, calendar);
}

// now we know endTime is the boundary immediately after v
// so startTime is obtained by incrementing backward one period.
var startTime = incrementMonth(endTime, -mPeriod, calendar);

newVals[i] = (
    isStart ? startTime :
    isEnd ? endTime :
    (startTime + endTime) / 2;
)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for your review.
Revised in 83b22a0.


var startTime, endTime;
if(dPeriod) {
startTime = Date.UTC(year, month, day);
endTime = startTime + dPeriod * ONEDAY;
}

if(mPeriod) {
var nYears = Math.floor(mPeriod / 12);
var nMonths = mPeriod % 12;

if(nMonths) {
startTime = Date.UTC(year, nYears ? month : roundMonth(month, nMonths));
endTime = incrementMonth(startTime, mPeriod, calendar);
} else {
startTime = Date.UTC(year, 0);
endTime = Date.UTC(year + nYears, 0);
}
}

var newD = new Date(
isStart ? startTime :
isEnd ? endTime :
(startTime + endTime) / 2
);

newVals[i] = newD.getTime() + base;
}
return newVals;
};

var monthSteps = [2, 3, 4, 6];

function roundMonth(month, step) {
return (monthSteps.indexOf(step) === -1) ? month : Math.floor(month / step) * step;
}
1 change: 0 additions & 1 deletion src/plots/cartesian/set_convert.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ var ONEMIN = numConstants.ONEMIN;
var ONESEC = numConstants.ONESEC;

var axisIds = require('./axis_ids');

var constants = require('./constants');
var HOUR_PATTERN = constants.HOUR_PATTERN;
var WEEKDAY_PATTERN = constants.WEEKDAY_PATTERN;
Expand Down
7 changes: 7 additions & 0 deletions src/traces/bar/attributes.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,13 @@ module.exports = {
y0: scatterAttrs.y0,
dy: scatterAttrs.dy,

xperiod: scatterAttrs.xperiod,
yperiod: scatterAttrs.yperiod,
xperiod0: scatterAttrs.xperiod0,
yperiod0: scatterAttrs.yperiod0,
xperiodalignment: scatterAttrs.xperiodalignment,
yperiodalignment: scatterAttrs.yperiodalignment,

text: scatterAttrs.text,
texttemplate: texttemplateAttrs({editType: 'plot'}, {
keys: constants.eventDataKeys
Expand Down
16 changes: 13 additions & 3 deletions src/traces/bar/calc.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
'use strict';

var Axes = require('../../plots/cartesian/axes');
var alignPeriod = require('../../plots/cartesian/align_period');
var hasColorscale = require('../../components/colorscale/helpers').hasColorscale;
var colorscaleCalc = require('../../components/colorscale/calc');
var arraysToCalcdata = require('./arrays_to_calcdata');
Expand All @@ -17,18 +18,23 @@ var calcSelection = require('../scatter/calc_selection');
module.exports = function calc(gd, trace) {
var xa = Axes.getFromId(gd, trace.xaxis || 'x');
var ya = Axes.getFromId(gd, trace.yaxis || 'y');
var size, pos;
var size, pos, origPos;

var sizeOpts = {
msUTC: !!(trace.base || trace.base === 0)
};

var hasPeriod;
if(trace.orientation === 'h') {
size = xa.makeCalcdata(trace, 'x', sizeOpts);
pos = ya.makeCalcdata(trace, 'y');
origPos = ya.makeCalcdata(trace, 'y');
pos = alignPeriod(trace, ya, 'y', origPos);
hasPeriod = !!trace.yperiodalignment;
} else {
size = ya.makeCalcdata(trace, 'y', sizeOpts);
pos = xa.makeCalcdata(trace, 'x');
origPos = xa.makeCalcdata(trace, 'x');
pos = alignPeriod(trace, xa, 'x', origPos);
hasPeriod = !!trace.xperiodalignment;
}

// create the "calculated data" to plot
Expand All @@ -39,6 +45,10 @@ module.exports = function calc(gd, trace) {
for(var i = 0; i < serieslen; i++) {
cd[i] = { p: pos[i], s: size[i] };

if(hasPeriod) {
cd[i].orig_p = origPos[i]; // used by hover
}

if(trace.ids) {
cd[i].id = String(trace.ids[i]);
}
Expand Down
3 changes: 3 additions & 0 deletions src/traces/bar/defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ var Color = require('../../components/color');
var Registry = require('../../registry');

var handleXYDefaults = require('../scatter/xy_defaults');
var handlePeriodDefaults = require('../scatter/period_defaults');
var handleStyleDefaults = require('./style_defaults');
var getAxisGroup = require('../../plots/cartesian/axis_ids').getAxisGroup;
var attributes = require('./attributes');
Expand All @@ -30,6 +31,8 @@ function supplyDefaults(traceIn, traceOut, defaultColor, layout) {
return;
}

handlePeriodDefaults(traceIn, traceOut, layout, coerce);

coerce('orientation', (traceOut.x && !traceOut.y) ? 'h' : 'v');
coerce('base');
coerce('offset');
Expand Down
4 changes: 3 additions & 1 deletion src/traces/bar/hover.js
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,9 @@ function hoverOnBars(pointData, xval, yval, hovermode) {
var extent = t.extents[t.extents.round(di.p)];
pointData[posLetter + '0'] = pa.c2p(isClosest ? minPos(di) : extent[0], true);
pointData[posLetter + '1'] = pa.c2p(isClosest ? maxPos(di) : extent[1], true);
pointData[posLetter + 'LabelVal'] = di.p;

var hasPeriod = di.orig_p !== undefined;
pointData[posLetter + 'LabelVal'] = hasPeriod ? di.orig_p : di.p;

pointData.labelLabel = hoverLabelText(pa, pointData[posLetter + 'LabelVal']);
pointData.valueLabel = hoverLabelText(sa, pointData[sizeLetter + 'LabelVal']);
Expand Down
7 changes: 7 additions & 0 deletions src/traces/box/attributes.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,13 @@ module.exports = {
].join(' ')
},

xperiod: scatterAttrs.xperiod,
yperiod: scatterAttrs.yperiod,
xperiod0: scatterAttrs.xperiod0,
yperiod0: scatterAttrs.yperiod0,
xperiodalignment: scatterAttrs.xperiodalignment,
yperiodalignment: scatterAttrs.yperiodalignment,

name: {
valType: 'string',
role: 'info',
Expand Down
19 changes: 15 additions & 4 deletions src/traces/box/calc.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
var isNumeric = require('fast-isnumeric');

var Axes = require('../../plots/cartesian/axes');
var alignPeriod = require('../../plots/cartesian/align_period');
var Lib = require('../../lib');

var BADNUM = require('../../constants/numerical').BADNUM;
Expand All @@ -29,19 +30,24 @@ module.exports = function calc(gd, trace) {
var valAxis, valLetter;
var posAxis, posLetter;

var hasPeriod;
if(trace.orientation === 'h') {
valAxis = xa;
valLetter = 'x';
posAxis = ya;
posLetter = 'y';
hasPeriod = !!trace.yperiodalignment;
} else {
valAxis = ya;
valLetter = 'y';
posAxis = xa;
posLetter = 'x';
hasPeriod = !!trace.xperiodalignment;
}

var posArray = getPos(trace, posLetter, posAxis, fullLayout[numKey]);
var allPosArrays = getPosArrays(trace, posLetter, posAxis, fullLayout[numKey]);
var posArray = allPosArrays[0];
var origPos = allPosArrays[1];
var dv = Lib.distinctVals(posArray);
var posDistinct = dv.vals;
var dPos = dv.minDiff / 2;
Expand Down Expand Up @@ -77,6 +83,9 @@ module.exports = function calc(gd, trace) {

cdi = {};
cdi.pos = cdi[posLetter] = posi;
if(hasPeriod && origPos) {
cdi.orig_p = origPos[i]; // used by hover
}

cdi.q1 = d2c('q1');
cdi.med = d2c('median');
Expand Down Expand Up @@ -303,13 +312,15 @@ module.exports = function calc(gd, trace) {
// so if you want one box
// per trace, set x0 (y0) to the x (y) value or category for this trace
// (or set x (y) to a constant array matching y (x))
function getPos(trace, posLetter, posAxis, num) {
function getPosArrays(trace, posLetter, posAxis, num) {
var hasPosArray = posLetter in trace;
var hasPos0 = posLetter + '0' in trace;
var hasPosStep = 'd' + posLetter in trace;

if(hasPosArray || (hasPos0 && hasPosStep)) {
return posAxis.makeCalcdata(trace, posLetter);
var origPos = posAxis.makeCalcdata(trace, posLetter);
var pos = alignPeriod(trace, posAxis, posLetter, origPos);
return [pos, origPos];
}

var pos0;
Expand Down Expand Up @@ -337,7 +348,7 @@ function getPos(trace, posLetter, posAxis, num) {
var out = new Array(len);
for(var i = 0; i < len; i++) out[i] = pos0c;

return out;
return [out];
}

function makeBins(x, dx) {
Expand Down
3 changes: 3 additions & 0 deletions src/traces/box/defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
var Lib = require('../../lib');
var Registry = require('../../registry');
var Color = require('../../components/color');
var handlePeriodDefaults = require('../scatter/period_defaults');
var handleGroupingDefaults = require('../bar/defaults').handleGroupingDefaults;
var autoType = require('../../plots/cartesian/axis_autotype');
var attributes = require('./attributes');
Expand All @@ -23,6 +24,8 @@ function supplyDefaults(traceIn, traceOut, defaultColor, layout) {
handleSampleDefaults(traceIn, traceOut, coerce, layout);
if(traceOut.visible === false) return;

handlePeriodDefaults(traceIn, traceOut, layout, coerce);

var hasPreCompStats = traceOut._hasPreCompStats;

if(hasPreCompStats) {
Expand Down
8 changes: 5 additions & 3 deletions src/traces/box/hover.js
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ function hoverOnBoxes(pointData, xval, yval, hovermode) {
pointData[pLetter + '0'] = pAxis.c2p(di.pos + t.bPos - boxDeltaNeg, true);
pointData[pLetter + '1'] = pAxis.c2p(di.pos + t.bPos + boxDeltaPos, true);

pointData[pLetter + 'LabelVal'] = di.pos;
pointData[pLetter + 'LabelVal'] = di.orig_p !== undefined ? di.orig_p : di.pos;

var spikePosAttr = pLetter + 'Spike';
pointData.spikeDistance = dxy(di) * spikePseudoDistance / hoverPseudoDistance;
Expand Down Expand Up @@ -257,14 +257,16 @@ function hoverOnPoints(pointData, xval, yval) {
hovertemplate: trace.hovertemplate
});

var origPos = di.orig_p;
var pos = origPos !== undefined ? origPos : di.pos;
var pa;
if(trace.orientation === 'h') {
pa = ya;
closePtData.xLabelVal = pt.x;
closePtData.yLabelVal = di.pos;
closePtData.yLabelVal = pos;
} else {
pa = xa;
closePtData.xLabelVal = di.pos;
closePtData.xLabelVal = pos;
closePtData.yLabelVal = pt.y;
}

Expand Down
4 changes: 4 additions & 0 deletions src/traces/candlestick/attributes.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@ function directionAttrs(lineColorDefault) {
}

module.exports = {
xperiod: OHLCattrs.xperiod,
xperiod0: OHLCattrs.xperiod0,
xperiodalignment: OHLCattrs.xperiodalignment,

x: OHLCattrs.x,
open: OHLCattrs.open,
high: OHLCattrs.high,
Expand Down
6 changes: 4 additions & 2 deletions src/traces/candlestick/calc.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

var Lib = require('../../lib');
var Axes = require('../../plots/cartesian/axes');
var alignPeriod = require('../../plots/cartesian/align_period');

var calcCommon = require('../ohlc/calc').calcCommon;

Expand All @@ -18,9 +19,10 @@ module.exports = function(gd, trace) {
var xa = Axes.getFromId(gd, trace.xaxis);
var ya = Axes.getFromId(gd, trace.yaxis);

var x = xa.makeCalcdata(trace, 'x');
var origX = xa.makeCalcdata(trace, 'x');
var x = alignPeriod(trace, xa, 'x', origX);

var cd = calcCommon(gd, trace, x, ya, ptFunc);
var cd = calcCommon(gd, trace, origX, x, ya, ptFunc);

if(cd.length) {
Lib.extendFlat(cd[0].t, {
Expand Down
3 changes: 3 additions & 0 deletions src/traces/candlestick/defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
var Lib = require('../../lib');
var Color = require('../../components/color');
var handleOHLC = require('../ohlc/ohlc_defaults');
var handlePeriodDefaults = require('../scatter/period_defaults');
var attributes = require('./attributes');

module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout) {
Expand All @@ -25,6 +26,8 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
return;
}

handlePeriodDefaults(traceIn, traceOut, layout, coerce, {x: true});

coerce('line.width');

handleDirection(traceIn, traceOut, coerce, 'increasing');
Expand Down
8 changes: 8 additions & 0 deletions src/traces/contour/attributes.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,14 @@ module.exports = extendFlat({
y: heatmapAttrs.y,
y0: heatmapAttrs.y0,
dy: heatmapAttrs.dy,

xperiod: heatmapAttrs.xperiod,
yperiod: heatmapAttrs.yperiod,
xperiod0: scatterAttrs.xperiod0,
yperiod0: scatterAttrs.yperiod0,
xperiodalignment: heatmapAttrs.xperiodalignment,
yperiodalignment: heatmapAttrs.yperiodalignment,

text: heatmapAttrs.text,
hovertext: heatmapAttrs.hovertext,
transpose: heatmapAttrs.transpose,
Expand Down
3 changes: 3 additions & 0 deletions src/traces/contour/defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
var Lib = require('../../lib');

var handleXYZDefaults = require('../heatmap/xyz_defaults');
var handlePeriodDefaults = require('../scatter/period_defaults');
var handleConstraintDefaults = require('./constraint_defaults');
var handleContoursDefaults = require('./contours_defaults');
var handleStyleDefaults = require('./style_defaults');
Expand All @@ -32,6 +33,8 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
return;
}

handlePeriodDefaults(traceIn, traceOut, layout, coerce);

coerce('text');
coerce('hovertext');
coerce('hovertemplate');
Expand Down
Loading