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 3 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
29 changes: 22 additions & 7 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 @@ -777,20 +781,31 @@ axes.calcTicks = function calcTicks(ax, opts) {
var A = tickVals[a].value;
var B = tickVals[b].value;

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);
var actualDelta = Math.abs(B - A);
var delta = definedDelta || actualDelta;
if(delta >= ONEMINYEAR) {
if(actualDelta >= ONEMINYEAR && actualDelta <= ONEMAXYEAR) {
v += actualDelta / 2;
} else {
v += ONEAVGYEAR / 2;
}
} else if(delta >= ONEMINQUARTER) {
if(actualDelta >= ONEMINQUARTER && actualDelta <= ONEMAXQUARTER) {
v += actualDelta / 2;
} else {
v += ONEAVGQUARTER / 2;
}
} else if(delta >= ONEMINMONTH) {
if(actualDelta >= ONEMINMONTH && actualDelta <= ONEMAXMONTH) {
v += actualDelta / 2;
} else {
v += ONEAVGMONTH / 2;
}
} else if(delta >= ONEWEEK) {
v += ONEWEEK / 2;
if(actualDelta === ONEWEEK && ax._hasDayOfWeekBreaks) {
v -= ONEDAY; // half of two days which is a good approximate for the number of week-end days
Copy link
Collaborator

Choose a reason for hiding this comment

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

I forget, did we already shift the ticks to look like they're right at the end of the rangebreak? If so (please confirm) this seems like it'll make sense for 5-day work weeks regardless of which two days were removed and whether weeks start on Sunday or Monday. But there are still a few cases I'm worried about:

  • removing just one weekend day (eg Saturday is still included, just Sunday is removed)
  • weekend AND nightly breaks - the shift wouldn't be correct, and could even result in a broken tick because it lands within another rangebreak
  • holidays that shorten to a 4-day work week

The only thing I can think of is perhaps figure out the actual range of the week between A and B, go to the midpoint of THAT, and then check if the result is in a break and needs further shifting. Is that possible?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I forget, did we already shift the ticks to look like they're right at the end of the rangebreak? If so (please confirm)

Yes. The logic for setting rangebreaks labels is just above this part:

plotly.js/src/plots/cartesian/axes.js

Lines 640 to 682 in 8928851

if(ax.rangebreaks) {
// replace ticks inside breaks that would get a tick
// and reduce ticks
var len = tickVals.length;
if(len) {
var tf = 0;
if(ax.tickmode === 'auto') {
tf =
(ax._id.charAt(0) === 'y' ? 2 : 6) *
(ax.tickfont ? ax.tickfont.size : 12);
}
var newTickVals = [];
var prevPos;
var dir = axrev ? 1 : -1;
var first = axrev ? 0 : len - 1;
var last = axrev ? len - 1 : 0;
for(var q = first; dir * q <= dir * last; q += dir) {
var tickVal = tickVals[q];
if(ax.maskBreaks(tickVal.value) === BADNUM) {
tickVal.value = moveOutsideBreak(tickVal.value, ax);
if(ax._rl && (
ax._rl[0] === tickVal.value ||
ax._rl[1] === tickVal.value
)) continue;
}
var pos = ax.c2p(tickVal.value);
if(pos === prevPos) {
if(newTickVals[newTickVals.length - 1].value < tickVal.value) {
newTickVals[newTickVals.length - 1] = tickVal;
}
} else if(prevPos === undefined || Math.abs(pos - prevPos) > tf) {
prevPos = pos;
newTickVals.push(tickVal);
}
}
tickVals = newTickVals.reverse();
}
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

there are still a few cases I'm worried about:

  • removing just one weekend day (eg Saturday is still included, just Sunday is removed)
  • weekend AND nightly breaks - the shift wouldn't be correct, and could even result in a broken tick because it lands within another rangebreak
  • holidays that shorten to a 4-day work week

The only thing I can think of is perhaps figure out the actual range of the week between A and B, go to the midpoint of THAT, and then check if the result is in a break and needs further shifting. Is that possible?

Good call. I am working on a commit to help fix these cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed in f907f2a.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Weekend AND nightly breaks is still a problem: https://codepen.io/alexcjohnson/pen/yLOVjJy?editors=0010
Screen Shot 2020-08-21 at 12 04 30 PM

If you want to make this a separate issue and address it later, that's fine by me. If you want to go that way please file the issue and then I'll resolve this comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Weekend AND nightly breaks is still a problem: https://codepen.io/alexcjohnson/pen/yLOVjJy?editors=0010
Screen Shot 2020-08-21 at 12 04 30 PM

Good catch. This one addressed in 50a3451.
Here is a codepen.

}
} else if(delta >= ONEDAY) {
v += ONEDAY / 2;
}
Expand Down
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.
97 changes: 79 additions & 18 deletions test/jasmine/tests/axes_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5232,14 +5232,14 @@ describe('Test axes', function() {
})
.then(function() {
_assert('', [
'2019-07-02 15:00',
'2020-07-01 15:00',
'2021-07-02 15:00',
'2022-07-02 15:00',
'2023-07-02 15:00',
'2024-07-01 15:00',
'2025-07-02 15:00',
'2026-07-02 15:00'
'2019-07-02 12:00',
'2020-07-02',
'2021-07-02 12:00',
'2022-07-02 12:00',
'2023-07-02 12:00',
'2024-07-02',
'2025-07-02 12:00',
'2026-07-02 12:00'
], [
['', '2020', '2021', '2022', '2023', '2024', '2025', ''],
['', '20', '21', '22', '23', '24', '25', '']
Expand All @@ -5265,16 +5265,16 @@ describe('Test axes', function() {
})
.then(function() {
_assert('', [
'2019-11-15 15:45',
'2020-02-15 15:45',
'2020-05-16 15:45',
'2020-08-15 15:45',
'2020-11-15 15:45',
'2021-02-15 15:45',
'2021-05-16 15:45',
'2021-08-15 15:45',
'2021-11-15 15:45',
'2022-02-15 15:45'
'2019-11-16',
'2020-02-15 12:00',
'2020-05-16 12:00',
'2020-08-16',
'2020-11-16',
'2021-02-15',
'2021-05-16 12:00',
'2021-08-16',
'2021-11-16',
'2022-02-16'
], ['', '2020-1', '2020-2', '2020-3', '2020-4', '2021-1', '2021-2', '2021-3', '2021-4', '']);
})
.catch(failTest)
Expand Down Expand Up @@ -5383,6 +5383,67 @@ describe('Test axes', function() {
});
});

['%U', '%V', '%W'].forEach(function(tickformat, i) {
it('should move weekly labels by one day (i.e. to help center the labels) when *day of week* rangebreak is present', function(done) {
Plotly.newPlot(gd, {
data: [{
x: [
'2020-01-01',
'2020-01-02',
'2020-01-03',
'2020-01-04',
'2020-01-05',
'2020-01-06',
'2020-01-07',
'2020-01-08',
'2020-01-09',
'2020-01-10',
'2020-01-11',
'2020-01-12',
'2020-01-13',
'2020-01-14',
'2020-01-15',
'2020-01-16',
'2020-01-17',
'2020-01-18',
'2020-01-19',
'2020-01-20',
'2020-01-21',
'2020-01-22',
'2020-01-23',
'2020-01-24',
'2020-01-25',
'2020-01-26',
'2020-01-27',
'2020-01-28',
'2020-01-29',
'2020-01-30',
'2020-01-31'
]
}],
layout: {
width: 1000,
xaxis: {
rangebreaks: [{bounds: ['sat', 'mon']}],
ticklabelmode: 'period',
tickformat: '%b-' + tickformat
}
}
})
.then(function() {
_assert('', [
'2020-01-01 12:00', '2020-01-08 12:00', '2020-01-15 12:00', '2020-01-22 12:00', '2020-01-29 12:00'
], [
['Dec-52', 'Jan-01', 'Jan-02', 'Jan-03', 'Jan-04'],
['Dec-01', 'Jan-02', 'Jan-03', 'Jan-04', 'Jan-05'],
['Dec-52', 'Jan-01', 'Jan-02', 'Jan-03', 'Jan-04']
][i]);
})
.catch(failTest)
.then(done);
});
});

['%A', '%a', '%d', '%e', '%j', '%u', '%w', '%x'].forEach(function(tickformat, i) {
it('should respect daily tickformat that includes ' + tickformat, function(done) {
Plotly.newPlot(gd, {
Expand Down