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

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 18 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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion src/constants/numerical.js
View file
Open in desktop
Original file line number Diff line number Diff line change
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
113 changes: 76 additions & 37 deletions src/plots/cartesian/axes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,12 @@ 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;
Expand Down Expand Up @@ -698,9 +702,10 @@ 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))
!(/%[fLQsSMHIpX]/.test(tickformat))
// %f: microseconds as a decimal number [000000, 999999]
// %L: milliseconds as a decimal number [000, 999]
// %Q: milliseconds since UNIX epoch
Expand All @@ -713,7 +718,7 @@ axes.calcTicks = function calcTicks(ax, opts) {
// %X: the locale’s time, such as %-I:%M:%S %p
) {
if(
/%[Aadejuwx]/.test(ax.tickformat)
/%[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,93 +729,126 @@ 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;
}

if(periodLength && ax.rangebreaks) {
var nOut = 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) nOut++;
}
periodLength *= 1 - nOut / nAll;
}

v += periodLength / 2;

ticksOut[i].periodX = v;

if(v > maxRange || v < minRange) { // hide label if outside the range
ticksOut[i].text = '';
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 +962,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
Loading
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
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading