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

Jump to bottom

Fixup weekly and quarterly period label positioning and with rangebreaks #5089

Merged
merged 24 commits into from Sep 2, 2020
Merged

Fixup weekly and quarterly period label positioning and with rangebreaks #5089

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
1a09b58
center weekly labels on weekend rangebreaks
archmoj Aug 19, 2020
9fce7ad
improve quarters and year period label positioning
archmoj Aug 19, 2020
8928851
reduce min quarter constant for the edge case of Feb-Mar-Apr
archmoj Aug 19, 2020
ea6eca1
add logic for special case of weeks and quarters
archmoj Aug 19, 2020
37a0840
use tickfromat getter and add question comment
archmoj Aug 19, 2020
e0bd026
more fixups for period tickformat
archmoj Aug 19, 2020
6fb0133
remove comment
archmoj Aug 19, 2020
f907f2a
revise period lengths on axes with rangebreaks
archmoj Aug 19, 2020
fa5d2e3
do not recalculate periodLength when zero
archmoj Aug 19, 2020
b318e8b
reduce sampling
archmoj Aug 19, 2020
beed352
should not move by a week or quarter when the labels are not weekly o…
archmoj Aug 19, 2020
104ae5e
use formatter in the variable name of the entire tickformat
archmoj Aug 20, 2020
7febd59
add tests for auto labels and rangebreaks
archmoj Aug 20, 2020
636531b
add hovertemplate to make debugging easier in future
archmoj Aug 20, 2020
dddcaa8
make week and quarter labels mode readable in tests
archmoj Aug 20, 2020
b32b5a4
add tests for period labels on axes with reversed ranges
archmoj Aug 20, 2020
350a10a
position quarters in period mode - case of dtick set to M6
archmoj Aug 31, 2020
50a3451
skip labels with identical text as previous label
archmoj Aug 31, 2020
2920e31
ensure new label positions remains between ticks
archmoj Aug 31, 2020
12ce066
adjust AM/PM period case
archmoj Aug 31, 2020
f47c820
adjust hour period case
archmoj Aug 31, 2020
c080bc5
adjust period labels depending on break size
archmoj Sep 1, 2020
4c10ddd
adjust period labels for the case of big gaps
archmoj Sep 2, 2020
5775048
split sampling over first and second halves
archmoj Sep 2, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 5 additions & 1 deletion src/constants/numerical.js
View file
Open in desktop
Expand Up @@ -31,13 +31,17 @@ module.exports = {
* to remind us that not all years and months
* have the same length
*/
ONEMAXYEAR: 31622400000, // 366 * ONEDAY
ONEAVGYEAR: 31557600000, // 365.25 days
ONEMINYEAR: 31536000000, // 365 * ONEDAY
ONEMAXQUARTER: 7948800000, // 92 * ONEDAY
ONEAVGQUARTER: 7889400000, // 1/4 of ONEAVGYEAR
ONEMINQUARTER: 7689600000, // 89 * ONEDAY
ONEMAXMONTH: 2678400000, // 31 * ONEDAY
ONEAVGMONTH: 2629800000, // 1/12 of ONEAVGYEAR
ONEMINMONTH: 2419200000, // 28 * ONEDAY
ONEWEEK: 604800000, // 7 * ONEDAY
ONEDAY: 86400000,
ONEDAY: 86400000, // 24 * ONEHOUR
ONEHOUR: 3600000,
ONEMIN: 60000,
ONESEC: 1000,
Expand Down
143 changes: 102 additions & 41 deletions src/plots/cartesian/axes.js
View file
Open in desktop
Expand Up @@ -23,13 +23,18 @@ var axAttrs = require('./layout_attributes');
var cleanTicks = require('./clean_ticks');

var constants = require('../../constants/numerical');
var ONEMAXYEAR = constants.ONEMAXYEAR;
var ONEAVGYEAR = constants.ONEAVGYEAR;
var ONEMINYEAR = constants.ONEMINYEAR;
var ONEMAXQUARTER = constants.ONEMAXQUARTER;
var ONEAVGQUARTER = constants.ONEAVGQUARTER;
var ONEMINQUARTER = constants.ONEMINQUARTER;
var ONEMAXMONTH = constants.ONEMAXMONTH;
var ONEAVGMONTH = constants.ONEAVGMONTH;
var ONEMINMONTH = constants.ONEMINMONTH;
var ONEWEEK = constants.ONEWEEK;
var ONEDAY = constants.ONEDAY;
var HALFDAY = ONEDAY / 2;
var ONEHOUR = constants.ONEHOUR;
var ONEMIN = constants.ONEMIN;
var ONESEC = constants.ONESEC;
Expand Down Expand Up @@ -499,7 +504,7 @@ function autoShiftMonthBins(binStart, data, dtick, dataMin, calendar) {
// will always give a somewhat odd-looking label, until we do something
// smarter like showing the bin boundaries (or the bounds of the actual
// data in each bin)
binStart -= ONEDAY / 2;
binStart -= HALFDAY;
}
var nextBinStart = axes.tickIncrement(binStart, dtick);

Expand Down Expand Up @@ -698,22 +703,28 @@ axes.calcTicks = function calcTicks(ax, opts) {
var maxRange = Math.max(rng[0], rng[1]);

var definedDelta;
if(isPeriod && ax.tickformat) {
var tickformat = axes.getTickFormat(ax);
if(isPeriod && tickformat) {
if(
!(/%[fLQsSMHIpX]/.test(ax.tickformat))
!(/%[fLQsSMX]/.test(tickformat))
// %f: microseconds as a decimal number [000000, 999999]
// %L: milliseconds as a decimal number [000, 999]
// %Q: milliseconds since UNIX epoch
// %s: seconds since UNIX epoch
// %S: second as a decimal number [00,61]
// %M: minute as a decimal number [00,59]
// %H: hour (24-hour clock) as a decimal number [00,23]
// %I: hour (12-hour clock) as a decimal number [01,12]
// %p: either AM or PM
// %X: the locale’s time, such as %-I:%M:%S %p
) {
if(
/%[Aadejuwx]/.test(ax.tickformat)
/%[HI]/.test(tickformat)
// %H: hour (24-hour clock) as a decimal number [00,23]
// %I: hour (12-hour clock) as a decimal number [01,12]
) definedDelta = ONEHOUR;
else if(
/%p/.test(tickformat) // %p: either AM or PM
) definedDelta = HALFDAY;
else if(
/%[Aadejuwx]/.test(tickformat)
// %A: full weekday name
// %a: abbreviated weekday name
// %d: zero-padded day of the month as a decimal number [01,31]
Expand All @@ -724,75 +735,124 @@ axes.calcTicks = function calcTicks(ax, opts) {
// %x: the locale’s date, such as %-m/%-d/%Y
) definedDelta = ONEDAY;
else if(
/%[UVW]/.test(ax.tickformat)
/%[UVW]/.test(tickformat)
// %U: Sunday-based week of the year as a decimal number [00,53]
// %V: ISO 8601 week of the year as a decimal number [01, 53]
// %W: Monday-based week of the year as a decimal number [00,53]
) definedDelta = ONEWEEK;
else if(
/%[Bbm]/.test(ax.tickformat)
/%[Bbm]/.test(tickformat)
// %B: full month name
// %b: abbreviated month name
// %m: month as a decimal number [01,12]
) definedDelta = ONEAVGMONTH;
else if(
/%[q]/.test(ax.tickformat)
/%[q]/.test(tickformat)
// %q: quarter of the year as a decimal number [1,4]
) definedDelta = ONEAVGQUARTER;
else if(
/%[Yy]/.test(ax.tickformat)
/%[Yy]/.test(tickformat)
// %Y: year with century as a decimal number, such as 1999
// %y: year without century as a decimal number [00,99]
) definedDelta = ONEAVGYEAR;
}
}

var removedPreTick0Label = false;
var ticksOut = new Array(tickVals.length);
var ticksOut = [];
var i;
var prevText;
for(i = 0; i < tickVals.length; i++) {
var _minor = tickVals[i].minor;
var _value = tickVals[i].value;

ticksOut[i] = axes.tickText(
var t = axes.tickText(
ax,
_value,
false, // hover
_minor // noSuffixPrefix
);

if(isPeriod) {
var v = tickVals[i].value;
if(isPeriod && prevText === t.text) continue;
prevText = t.text;

ticksOut.push(t);
}

if(isPeriod) {
var removedPreTick0Label = false;

for(i = 0; i < ticksOut.length; i++) {
var v = ticksOut[i].x;

var a = i;
var b = i + 1;
if(i < tickVals.length - 1) {
if(i < ticksOut.length - 1) {
a = i;
b = i + 1;
} else {
} else if(i > 0) {
a = i - 1;
b = i;
} else {
a = i;
b = i;
}

var A = tickVals[a].value;
var B = tickVals[b].value;
var A = ticksOut[a].x;
var B = ticksOut[b].x;
var actualDelta = Math.abs(B - A);
var delta = definedDelta || actualDelta;
var periodLength = 0;

var delta = definedDelta || Math.abs(B - A);
if(delta >= ONEDAY * 365) { // Years could have days less than ONEAVGYEAR period
v += ONEAVGYEAR / 2;
} else if(delta >= ONEAVGQUARTER) {
v += ONEAVGQUARTER / 2;
} else if(delta >= ONEMINMONTH) { // Months could have days less than ONEAVGMONTH period
var actualDelta = Math.abs(B - A);
if(delta >= ONEMINYEAR) {
if(actualDelta >= ONEMINYEAR && actualDelta <= ONEMAXYEAR) {
periodLength = actualDelta;
} else {
periodLength = ONEAVGYEAR;
}
} else if(definedDelta === ONEAVGQUARTER && delta >= ONEMINQUARTER) {
if(actualDelta >= ONEMINQUARTER && actualDelta <= ONEMAXQUARTER) {
periodLength = actualDelta;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need an else periodLength = ONEAVGQUARTER here, to cover the case of quarter tickformat but longer dtick?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's the case I thought it could help.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

At first I thought we need an else blocks for weeks and quarters to make them similar to others.
But ( as you mentioned) since we don't generate auto labels for weeks and quarters that could lead to the problem of moving daily/monthly labels by a week/quarter. So those else blocks were removed in beed352 commit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do we need an else periodLength = ONEAVGQUARTER here, to cover the case of quarter tickformat but longer dtick?

No. Adding the else condition would result in wrong label positioning for the case of quarters.
So I think we could resolve this.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Here's the case I think this else would address: quarter period ticklabels, and dtick of 6 months or 1 year https://codepen.io/alexcjohnson/pen/abNBYxZ?editors=0010
Currently this puts the quarter label right on the tick:
Screen Shot 2020-08-21 at 11 53 23 AM
but just like when we have monthly labels and 3-month dtick and we shift the label half a month over anyway, we should still be shifting the label half a quarter in this case. If, in this codepen, you change dtick to "M3", exactly a quarter, then it does shift the label correctly:
Screen Shot 2020-08-21 at 11 53 40 AM

Note: if you turn rangebreaks back on in that codepen (notice I renamed to rangebreaksx to disable them) and disable the explicit dtick, then all manner of bad things happen as you zoom and pan - unpredictable and nonuniform tick spacing, strange jumps...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here's the case I think this else would address: quarter period ticklabels, and dtick of 6 months or 1 year https://codepen.io/alexcjohnson/pen/abNBYxZ?editors=0010
Currently this puts the quarter label right on the tick:
Screen Shot 2020-08-21 at 11 53 23 AM
but just like when we have monthly labels and 3-month dtick and we shift the label half a month over anyway, we should still be shifting the label half a quarter in this case.

Good catch. This one fixed by 350a10a.
Here is an updated codepen.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: if you turn rangebreaks back on in that codepen (notice I renamed to rangebreaksx to disable them) and disable the explicit dtick, then all manner of bad things happen as you zoom and pan - unpredictable and nonuniform tick spacing, strange jumps...

Now I also put rangebreaks back in the above pen; and it looks OK.

} else {
periodLength = ONEAVGQUARTER;
}
} else if(delta >= ONEMINMONTH) {
if(actualDelta >= ONEMINMONTH && actualDelta <= ONEMAXMONTH) {
v += actualDelta / 2;
periodLength = actualDelta;
} else {
v += ONEAVGMONTH / 2;
periodLength = ONEAVGMONTH;
}
} else if(delta >= ONEWEEK) {
v += ONEWEEK / 2;
} else if(definedDelta === ONEWEEK && delta >= ONEWEEK) {
periodLength = ONEWEEK;
} else if(delta >= ONEDAY) {
v += ONEDAY / 2;
periodLength = ONEDAY;
} else if(definedDelta === HALFDAY && delta >= HALFDAY) {
periodLength = HALFDAY;
} else if(definedDelta === ONEHOUR && delta >= ONEHOUR) {
periodLength = ONEHOUR;
}

if(periodLength && ax.rangebreaks) {
var nFirstHalf = 0;
var nSecondHalf = 0;
var nAll = 2 * 3 * 7; // number of samples
for(var c = 0; c < nAll; c++) {
var r = c / nAll;
if(ax.maskBreaks(A * (1 - r) + B * r) !== BADNUM) {
if(r < 0.5) {
nFirstHalf++;
} else {
nSecondHalf++;
}
}
}

if(nSecondHalf) {
periodLength *= (nFirstHalf + nSecondHalf) / nAll;
}
}

if(periodLength <= actualDelta) { // i.e. to ensure new label positions remain between ticks
v += periodLength / 2;
}

ticksOut[i].periodX = v;
Expand All @@ -802,15 +862,15 @@ axes.calcTicks = function calcTicks(ax, opts) {
removedPreTick0Label = true;
}
}
}

if(removedPreTick0Label) {
for(i = 0; i < ticksOut.length; i++) {
if(ticksOut[i].periodX <= maxRange && ticksOut[i].periodX >= minRange) {
// redo first visible tick
ax._prevDateHead = '';
ticksOut[i].text = axes.tickText(ax, tickVals[i].value).text;
break;
if(removedPreTick0Label) {
for(i = 0; i < ticksOut.length; i++) {
if(ticksOut[i].periodX <= maxRange && ticksOut[i].periodX >= minRange) {
// redo first visible tick
ax._prevDateHead = '';
ticksOut[i].text = axes.tickText(ax, ticksOut[i].x).text;
break;
}
}
}
}
Expand Down Expand Up @@ -924,7 +984,8 @@ axes.autoTicks = function(ax, roughDTick) {
// 2 or 3 days... but that's a weird enough case that we'll ignore it.
ax.tick0 = Lib.dateTick0(ax.calendar, true);

if(/%[uVW]/.test(ax.tickformat)) {
var tickformat = axes.getTickFormat(ax);
if(/%[uVW]/.test(tickformat)) {
// replace Sunday with Monday for ISO and Monday-based formats
var len = ax.tick0.length;
var lastD = +ax.tick0[len - 1];
Expand Down
Binary file modified test/image/baselines/date_axes_period.png
View file
Open in desktop
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/image/baselines/date_axes_period2.png
View file
Open in desktop
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.