Skip to content

Fix rangebreaks overlapping and tick positions #4831

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

Merged
merged 22 commits into from
Jun 3, 2020
Merged

Fix rangebreaks overlapping and tick positions #4831

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
c28d0e5
centralize & refactor range expansions
archmoj Apr 6, 2020
1e63bfc
add rangebreaks tests with hourly and auto dticks
archmoj May 13, 2020
3baed24
revise tick positioning - fix dtick hourly - move tick0 outside the b…
archmoj May 13, 2020
4eab137
drop endpoint ticks that fall inside breaks
archmoj May 14, 2020
a7633c3
move last tick on rangebreaks to the start of break instead of removi…
archmoj May 15, 2020
d2d8e6e
revisit ticks on axes with rangebreaks
archmoj May 19, 2020
ca193eb
reject last tick that may include decimals
archmoj May 19, 2020
6b77b31
refactor - no need for else ifs in axes tickIncrement
archmoj May 19, 2020
ef14d60
add jasmine tests for axes with rangebreaks and set dtick
archmoj May 19, 2020
a0d4d0d
improve auto ticks on axes with rangebreaks
archmoj May 20, 2020
0fb92d4
attempt improving ticks on rangebreaks
archmoj Jun 1, 2020
5c8055b
fix issue 4879
archmoj Jun 1, 2020
0a782d8
invert if statement
archmoj Jun 2, 2020
6cd55b5
drop extra function for hour rangebreaks
archmoj Jun 2, 2020
db7d47d
drop extra logic for dayRatio
archmoj Jun 2, 2020
8c8c928
drop extra logic for dynamic oneDay
archmoj Jun 2, 2020
21f4284
drop extra logic for startHour
archmoj Jun 2, 2020
9e7b80e
drop extra logic for dayHours
archmoj Jun 2, 2020
22e5658
avoid too tick overlaps on x axes with rangebreaks and now only when …
archmoj Jun 2, 2020
b38688d
reverse loop order
archmoj Jun 2, 2020
c5cf45a
fixups to avoid single tick
archmoj Jun 2, 2020
9241578
reduce roughDTick by total length of breaks
archmoj Jun 3, 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
125 changes: 86 additions & 39 deletions src/plots/cartesian/axes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,15 @@ var autorange = require('./autorange');
axes.getAutoRange = autorange.getAutoRange;
axes.findExtremes = autorange.findExtremes;

var epsilon = 0.0001;
function expandRange(range) {
var delta = (range[1] - range[0]) * epsilon;
return [
range[0] - delta,
range[1] + delta
];
}

/*
* find the list of possible axes to reference with an xref or yref attribute
* and coerce it to that list
Expand Down Expand Up @@ -527,7 +536,7 @@ axes.prepTicks = function(ax) {
if(ax.tickmode === 'array') nt *= 100;


ax._roughDTick = (Math.abs(rng[1] - rng[0]) - (ax._lBreaks || 0)) / nt;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looking much nicer now!

I'm just noticing that the auto dtick we get depends on whether there's a break within the visible range or not, which seems weird, and seems like exactly what this - ax._lBreaks was designed to avoid. For example (back on everyone's favorite, axes_breaks-candlestick2 😅) if I zoom in while keeping a break in range I can't get dtick less than 2 hours:
Screen Shot 2020-06-02 at 10 55 55 PM

But shift the break off the edge with exactly the same scale and it snaps to 15 min:
Screen Shot 2020-06-02 at 10 57 52 PM

Is there something bad that happens if this line is left as it was?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch. Addressed in 9241578.

ax._roughDTick = Math.abs(rng[1] - rng[0]) / nt;
axes.autoTicks(ax, ax._roughDTick);

// check for a forced minimum dtick
Expand Down Expand Up @@ -566,8 +575,9 @@ axes.calcTicks = function calcTicks(ax) {
ax._tmin = axes.tickFirst(ax);

// add a tiny bit so we get ticks which may have rounded out
var startTick = rng[0] * 1.0001 - rng[1] * 0.0001;
var endTick = rng[1] * 1.0001 - rng[0] * 0.0001;
var exRng = expandRange(rng);
var startTick = exRng[0];
var endTick = exRng[1];
// check for reversed axis
var axrev = (rng[1] < rng[0]);

Expand Down Expand Up @@ -612,42 +622,32 @@ axes.calcTicks = function calcTicks(ax) {

if(ax.rangebreaks) {
// replace ticks inside breaks that would get a tick
if(ax.tickmode === 'auto') {
for(var t = 0; t < tickVals.length; t++) {
var value = tickVals[t].value;
if(ax.maskBreaks(value) === BADNUM) {
// find which break we are in
for(var k = 0; k < ax._rangebreaks.length; k++) {
var brk = ax._rangebreaks[k];
if(value >= brk.min && value < brk.max) {
tickVals[t].value = brk.max; // replace with break end
break;
}
}
}
}
}

// reduce ticks
// and reduce ticks
var len = tickVals.length;
if(len > 2) {
var tf2 = 2 * (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) { // apply reverse loop to pick greater values in breaks first
var pos = ax.c2p(tickVals[q].value);
var dir = axrev ? -1 : 1;
var first = axrev ? len - 1 : 0;
var last = axrev ? 0 : len - 1;
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 && tickVal.value === ax._rl[1]) continue;
}

var pos = ax.c2p(tickVal.value);
if(prevPos === undefined || Math.abs(pos - prevPos) > tf2) {
prevPos = pos;
newTickVals.push(tickVals[q]);
newTickVals.push(tickVal);
}
}
tickVals = newTickVals.reverse();
tickVals = newTickVals;
}
}

Expand Down Expand Up @@ -691,10 +691,9 @@ function arrayTicks(ax) {
var text = ax.ticktext;
var ticksOut = new Array(vals.length);
var rng = Lib.simpleMap(ax.range, ax.r2l);
var r0expanded = rng[0] * 1.0001 - rng[1] * 0.0001;
var r1expanded = rng[1] * 1.0001 - rng[0] * 0.0001;
var tickMin = Math.min(r0expanded, r1expanded);
var tickMax = Math.max(r0expanded, r1expanded);
var exRng = expandRange(rng);
var tickMin = Math.min(exRng[0], exRng[1]);
var tickMax = Math.max(exRng[0], exRng[1]);
var j = 0;

// without a text array, just format the given values as any other ticks
Expand Down Expand Up @@ -732,6 +731,25 @@ function arrayTicks(ax) {
return ticksOut;
}

function roundBaseDay(dayHours) {
switch(dayHours) {
case 4: return [1, 2];
case 6: return [1, 2, 3];
case 8: return [1, 2, 4];
case 9: return [1, 3];
case 10: return [1, 2, 5];
case 12: return [1, 2, 3, 4, 6];
case 14: return [1, 2, 7];
case 15: return [1, 3, 5];
case 16: return [1, 2, 4, 8];
case 18: return [1, 2, 3, 6, 9];
case 20: return [1, 2, 4, 5, 10];
case 21: return [1, 3, 7];
case 22: return [1, 2, 11];
Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh interesting... so just a precise list of factors of the rounded number of visible hours. I'm not sure we really want all of these, and we might want to add something intermediate for the larger primes so we don't jump straight from 1 day to 1 hour, even though we can't do it precisely uniformly. But this is a great starting point anyway, we can tweak later.

However, looking at the mocks changed in this PR it seems like there's a problem when dtick is not an even fraction of a day. For example in axes_breaks-candlestick2, dtick is 18 hours (how did that happen? I only see 9 here, I would have thought it would go to 24 after that? That doesn't matter though, there are lots of periods we want that don't divide 24 evenly, this is just an example), and as a result some days have 2 ticks (at 9:30 and 12:00) and other days have only one (at 9:30).

Trying to figure out a reasonable way to handle this, in conjunction with tick0 possibly being set manually. I think what we may need to say is: when you have hourly rangebreaks and dtick < 24h, the only piece of tick0 that we consider is the time portion and we start at that time on each new day, then increment by dtick until we get to the next day, at which point we reset the time to tick0 again, so you get ticks at the same hours in every day.

}
return [1];
}

var roundBase10 = [2, 5, 10];
var roundBase24 = [1, 2, 3, 6, 12];
var roundBase60 = [1, 2, 5, 10, 15, 30];
Expand Down Expand Up @@ -777,22 +795,34 @@ axes.autoTicks = function(ax, roughDTick) {
// being > half of the final unit - so precalculate twice the rough val
var roughX2 = 2 * roughDTick;

var oneDay = ONEDAY;
var dayRatio = 1;
if(ax._hasHourBreaks) {
oneDay = ax._dayHours * ONEHOUR;
dayRatio = Math.round(ax._dayHours / 24 * 7) / 7; // we use this in week context
}

if(roughX2 > ONEAVGYEAR) {
roughDTick /= ONEAVGYEAR;
base = getBase(10);
ax.dtick = 'M' + (12 * roundDTick(roughDTick, base, roundBase10));
} else if(roughX2 > ONEAVGMONTH) {
roughDTick /= ONEAVGMONTH;
ax.dtick = 'M' + roundDTick(roughDTick, 1, roundBase24);
} else if(roughX2 > ONEDAY) {
ax.dtick = roundDTick(roughDTick, ONEDAY, ax._hasDayOfWeekBreaks ? [1, 7, 14] : roundDays);

} else if(roughX2 > oneDay) {
ax.dtick = roundDTick(roughDTick, oneDay, ax._hasDayOfWeekBreaks ?
[1, 2 * dayRatio, 7 * dayRatio, 14 * dayRatio] :
roundDays
);
// get week ticks on sunday
// this will also move the base tick off 2000-01-01 if dtick is
// 2 or 3 days... but that's a weird enough case that we'll ignore it.
ax.tick0 = Lib.dateTick0(ax.calendar, true);
} else if(roughX2 > ONEHOUR) {
ax.dtick = roundDTick(roughDTick, ONEHOUR, roundBase24);
ax.dtick = roundDTick(roughDTick, ONEHOUR, ax._hasHourBreaks ?
roundBaseDay(ax._dayHours) :
roundBase24
);
} else if(roughX2 > ONEMIN) {
ax.dtick = roundDTick(roughDTick, ONEMIN, roundBase60);
} else if(roughX2 > ONESEC) {
Expand Down Expand Up @@ -934,18 +964,20 @@ axes.tickIncrement = function(x, dtick, axrev, calendar) {
if(tType === 'M') return Lib.incrementMonth(x, dtSigned, calendar);

// Log scales: Linear, Digits
else if(tType === 'L') return Math.log(Math.pow(10, x) + dtSigned) / Math.LN10;
if(tType === 'L') return Math.log(Math.pow(10, x) + dtSigned) / Math.LN10;

// log10 of 2,5,10, or all digits (logs just have to be
// close enough to round)
else if(tType === 'D') {
if(tType === 'D') {
var tickset = (dtick === 'D2') ? roundLog2 : roundLog1;
var x2 = x + axSign * 0.01;
var frac = Lib.roundUp(Lib.mod(x2, 1), tickset, axrev);

return Math.floor(x2) +
Math.log(d3.round(Math.pow(10, frac), 1)) / Math.LN10;
} else throw 'unrecognized dtick ' + String(dtick);
}

throw 'unrecognized dtick ' + String(dtick);
};

// calculate the first tick on an axis
Expand All @@ -956,10 +988,14 @@ axes.tickFirst = function(ax) {
var sRound = axrev ? Math.floor : Math.ceil;
// add a tiny extra bit to make sure we get ticks
// that may have been rounded out
var r0 = rng[0] * 1.0001 - rng[1] * 0.0001;
var r0 = expandRange(rng)[0];
var dtick = ax.dtick;
var tick0 = r2l(ax.tick0);

if(ax.tickmode === 'auto' && ax.rangebreaks && ax.maskBreaks(tick0) === BADNUM) {
tick0 = moveOutsideBreak(tick0, ax);
}

if(isNumeric(dtick)) {
var tmin = sRound((r0 - tick0) / dtick) * dtick + tick0;

Expand Down Expand Up @@ -3158,3 +3194,14 @@ function swapAxisAttrs(layout, key, xFullAxes, yFullAxes, dfltTitle) {
function isAngular(ax) {
return ax._id === 'angularaxis';
}

function moveOutsideBreak(v, ax) {
var len = ax._rangebreaks.length;
for(var k = 0; k < len; k++) {
var brk = ax._rangebreaks[k];
if(v >= brk.min && v < brk.max) {
return brk.max;
}
}
return v;
}
15 changes: 13 additions & 2 deletions src/plots/cartesian/axis_defaults.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,11 +135,22 @@ module.exports = function handleAxisDefaults(containerIn, containerOut, coerce,
if(!containerOut.rangebreaks.length) {
delete containerOut.rangebreaks;
} else {
var n = 0;
for(var k = 0; k < containerOut.rangebreaks.length; k++) {
if(containerOut.rangebreaks[k].pattern === DAY_OF_WEEK) {
var brk = containerOut.rangebreaks[k];
if(brk.pattern === DAY_OF_WEEK) {
containerOut._hasDayOfWeekBreaks = true;
break;
n++;
}

if(brk.pattern === HOUR) {
containerOut._hasHourBreaks = true;
containerOut._dayHours = Math.round((brk.bounds[1] - brk.bounds[0] + 24) % 24);
n++;
}

// break when found all types
if(n === 2) break;
Copy link
Collaborator

Choose a reason for hiding this comment

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

what if the user includes two breaks of one type, THEN a break of the other type? I suppose I can theoretically imagine wanting two disjoint hour periods - a morning shift and an evening shift, with a midday break? I wouldn't worry about doing something "pretty" in this case, perhaps just set _dayHours to something like 1 or 2 so we go straight from day to hour ticks or perhaps just use the first break. But it definitely shouldn't stop us from noticing _hasDayOfWeekBreaks

And vice versa I guess... maybe someone wants to skip Sundays and Wednesdays? Again, weird, but shouldn't stop us from catching _hasHourBreaks

}

setConvert(containerOut, layoutOut);
Expand Down
Binary file modified test/image/baselines/axes_breaks-candlestick2.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/axes_breaks-contour1d.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/axes_breaks-contour2d.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 added test/image/baselines/axes_breaks-dtick_auto.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 added test/image/baselines/axes_breaks-dtick_hourly.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/axes_breaks-finance.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/axes_breaks-heatmap1d.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/axes_breaks-heatmap2d.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/axes_breaks-histogram2d.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/axes_breaks-night_autorange-reversed.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/axes_breaks-rangeslider.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/axes_breaks-reversed-without-pattern.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/axes_breaks-round-weekdays.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/axes_breaks-values.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/axes_breaks-weekends-weeknights.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/axes_breaks-weekends_autorange-reversed.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/axes_breaks.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.
115 changes: 115 additions & 0 deletions test/image/mocks/axes_breaks-dtick_auto.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
{
"data": [
{
"type": "scatter",
"mode": "markers",
"x": [
"2020-04-06T09:00:00",
"2020-04-06T10:00:00",
"2020-04-06T11:00:00",
"2020-04-06T12:00:00",
"2020-04-06T13:00:00",
"2020-04-06T14:00:00",
"2020-04-06T15:00:00",
"2020-04-06T16:00:00",
"2020-04-07T09:00:00",
"2020-04-07T10:00:00",
"2020-04-07T11:00:00",
"2020-04-07T12:00:00",
"2020-04-07T13:00:00",
"2020-04-07T14:00:00",
"2020-04-07T15:00:00",
"2020-04-07T16:00:00",
"2020-04-08T09:00:00",
"2020-04-08T10:00:00",
"2020-04-08T11:00:00",
"2020-04-08T12:00:00",
"2020-04-08T13:00:00",
"2020-04-08T14:00:00",
"2020-04-08T15:00:00",
"2020-04-08T16:00:00",
"2020-04-09T09:00:00",
"2020-04-09T10:00:00",
"2020-04-09T11:00:00",
"2020-04-09T12:00:00",
"2020-04-09T13:00:00",
"2020-04-09T14:00:00",
"2020-04-09T15:00:00",
"2020-04-09T16:00:00",
"2020-04-10T09:00:00",
"2020-04-10T10:00:00",
"2020-04-10T11:00:00",
"2020-04-10T12:00:00",
"2020-04-10T13:00:00",
"2020-04-10T14:00:00",
"2020-04-10T15:00:00",
"2020-04-10T16:00:00"
],
"y": [
0.19742877314456364,
0.1509714558226325,
0.3730270552929804,
0.7394093812216096,
1.2149308862244954,
1.5707342286171064,
1.0824483128021085,
0.9424263772804724,
1.1724169397045303,
0.8440466169659708,
0.8650832231701001,
0.41942121150935374,
0.11941773640575382,
-0.3620604691336322,
-0.06836276577621159,
-0.34443807771583146,
-0.49908639701892876,
-0.07100510355333789,
0.13340929837019488,
-0.33475177209849727,
-0.6700576156005845,
-0.548579214100821,
-0.4713506254966534,
-0.7334578041221448,
-0.299243806197351,
-0.18527785023145504,
-0.14964504720649674,
-0.05973507085192575,
0.17038695866484388,
-0.01766804585555426,
-0.11944698363946238,
-0.40960323466434023,
-0.7234102287840041,
-0.2790378388000705,
-0.039487043750782935,
-0.04902823513321586,
-0.3216136071598926,
-0.5672571253894997,
-1.009227965065624,
-1.0748113395075032
]
}
],
"layout": {
"width": 800,
"height": 600,
"xaxis": {
"rangebreaks": [
{
"bounds": [
17,
9
],
"pattern": "hour"
}
],
"title": {
"text": "time"
}
},
"yaxis": {
"title": {
"text": "values"
}
}
}
}
Loading