Skip to content

Improve axis tick increment #4070

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

Closed
wants to merge 10 commits into from
Closed

Improve axis tick increment #4070

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
3623224
add mock to test issue 3814
archmoj Jul 22, 2019
0a07168
before fixing issue 3814
archmoj Jul 22, 2019
4855033
fixing bug issue 3814 - use integers to increment aixs ticks
archmoj Jul 22, 2019
740574b
add new test includes d3 p & s formats as well as dates and negative …
archmoj Jul 23, 2019
916bf80
add incrementNumeric to Lib make it reusable and testable
archmoj Jul 23, 2019
86b135d
added jasmine tests for incrementNumeric
archmoj Jul 23, 2019
ab2facf
use scale instead of magic - only apply scale >= 1 - add more tests
archmoj Jul 23, 2019
76b0368
improve on edge cases and test coverage
archmoj Jul 29, 2019
7f585ce
Merge branch 'master' into fix3814-percent-formatting to resolve conf…
archmoj Mar 15, 2020
14f704b
update header 2019 > 2020
archmoj Mar 15, 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
38 changes: 38 additions & 0 deletions src/lib/increment_numeric.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/**
* 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';

module.exports = function incrementNumeric(x, delta) {
if(!delta) return x;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

One may simply return x + delta here to find out various failing tests.

// Note 1:
// 0.3 != 0.1 + 0.2 == 0.30000000000000004
// but 0.3 == (10 * 0.1 + 10 * 0.2) / 10
// Attempt to use integer steps to increment
var scale = 1 / Math.abs(delta);
if(scale < 1) scale = 1;
var newX = (
scale * x +
scale * delta
) / scale;

// Note 2:
// now we may also consider rounding to cover few more edge cases
// e.g. 0.3 * 3 = 0.8999999999999999
var lenDt = ('' + delta).length;
var lenX0 = ('' + x).length;
var lenX1 = ('' + newX).length;

if(lenX1 >= lenX0 + lenDt) { // likely a rounding error!
newX = +parseFloat(newX).toPrecision(12);
}

return newX;
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for adding those tests @archmoj - I understand (a little bit) more what you're attempting.

That said, I feel like we should be extending numFormat:

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

Lines 1311 to 1408 in 96fe262

function numFormat(v, ax, fmtoverride, hover) {
var isNeg = v < 0;
// max number of digits past decimal point to show
var tickRound = ax._tickround;
var exponentFormat = fmtoverride || ax.exponentformat || 'B';
var exponent = ax._tickexponent;
var tickformat = axes.getTickFormat(ax);
var separatethousands = ax.separatethousands;
// special case for hover: set exponent just for this value, and
// add a couple more digits of precision over tick labels
if(hover) {
// make a dummy axis obj to get the auto rounding and exponent
var ah = {
exponentformat: exponentFormat,
dtick: ax.showexponent === 'none' ? ax.dtick :
(isNumeric(v) ? Math.abs(v) || 1 : 1),
// if not showing any exponents, don't change the exponent
// from what we calculate
range: ax.showexponent === 'none' ? ax.range.map(ax.r2d) : [0, v || 1]
};
autoTickRound(ah);
tickRound = (Number(ah._tickround) || 0) + 4;
exponent = ah._tickexponent;
if(ax.hoverformat) tickformat = ax.hoverformat;
}
if(tickformat) return ax._numFormat(tickformat)(v).replace(/-/g, MINUS_SIGN);
// 'epsilon' - rounding increment
var e = Math.pow(10, -tickRound) / 2;
// exponentFormat codes:
// 'e' (1.2e+6, default)
// 'E' (1.2E+6)
// 'SI' (1.2M)
// 'B' (same as SI except 10^9=B not G)
// 'none' (1200000)
// 'power' (1.2x10^6)
// 'hide' (1.2, use 3rd argument=='hide' to eg
// only show exponent on last tick)
if(exponentFormat === 'none') exponent = 0;
// take the sign out, put it back manually at the end
// - makes cases easier
v = Math.abs(v);
if(v < e) {
// 0 is just 0, but may get exponent if it's the last tick
v = '0';
isNeg = false;
} else {
v += e;
// take out a common exponent, if any
if(exponent) {
v *= Math.pow(10, -exponent);
tickRound += exponent;
}
// round the mantissa
if(tickRound === 0) v = String(Math.floor(v));
else if(tickRound < 0) {
v = String(Math.round(v));
v = v.substr(0, v.length + tickRound);
for(var i = tickRound; i < 0; i++) v += '0';
} else {
v = String(v);
var dp = v.indexOf('.') + 1;
if(dp) v = v.substr(0, dp + tickRound).replace(/\.?0+$/, '');
}
// insert appropriate decimal point and thousands separator
v = Lib.numSeparate(v, ax._separators, separatethousands);
}
// add exponent
if(exponent && exponentFormat !== 'hide') {
if(isSIFormat(exponentFormat) && beyondSI(exponent)) exponentFormat = 'power';
var signedExponent;
if(exponent < 0) signedExponent = MINUS_SIGN + -exponent;
else if(exponentFormat !== 'power') signedExponent = '+' + exponent;
else signedExponent = String(exponent);
if(exponentFormat === 'e' || exponentFormat === 'E') {
v += exponentFormat + signedExponent;
} else if(exponentFormat === 'power') {
v += '×10<sup>' + signedExponent + '</sup>';
} else if(exponentFormat === 'B' && exponent === 9) {
v += 'B';
} else if(isSIFormat(exponentFormat)) {
v += SIPREFIXES[exponent / 3 + 5];
}
}
// put sign back in and return
// replace standard minus character (which is technically a hyphen)
// with a true minus sign
if(isNeg) return MINUS_SIGN + v;
return v;
}

where instead of exiting early

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

Line 1338 in 96fe262

if(tickformat) return ax._numFormat(tickformat)(v).replace(/-/g, MINUS_SIGN);

for when tickformat is set, we could construct the formatted number ourselves for tickformat values that include s and p. To me, that sounds a lot more robust than these floating-point tricks that gets us a rounded-off tick step. What do you think?

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 think it would be interesting to fix the issue at the root cause & thanks to JavaScript.
These rounding issues happen in other languages too.
Following images illustrate examples in Fortran & C++.
fortran

cpp

Copy link
Contributor

Choose a reason for hiding this comment

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

it would be interesting to fix the issue at the root cause

What is the root cause in your mind here?

};
2 changes: 2 additions & 0 deletions src/lib/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,8 @@ lib.clearResponsive = require('./clear_responsive');

lib.makeTraceGroups = require('./make_trace_groups');

lib.incrementNumeric = require('./increment_numeric');

lib._ = require('./localize');

lib.notifier = require('./notifier');
Expand Down
2 changes: 1 addition & 1 deletion src/plots/cartesian/axes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -907,7 +907,7 @@ axes.tickIncrement = function(x, dtick, axrev, calendar) {
var axSign = axrev ? -1 : 1;

// includes linear, all dates smaller than month, and pure 10^n in log
if(isNumeric(dtick)) return x + axSign * dtick;
if(isNumeric(dtick)) return Lib.incrementNumeric(x, axSign * dtick);

// everything else is a string, one character plus a number
var tType = dtick.charAt(0);
Expand Down
Binary file added test/image/baselines/tick-increment.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/tick_percent.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.
136 changes: 136 additions & 0 deletions test/image/mocks/tick-increment.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
{
"data": [
{
"name": "s & positive",
"x": [
"2019-12-31 23:59:59.998",
"2020-01-01 00:00:00",
"2020-01-01 00:00:00.002"
],
"y": [
"1e-1",
"1e-2",
"1e-0"
],
"type": "scatter"
},
{
"name": "s & negative",
"xaxis": "x2",
"yaxis": "y2",
"x": [
"2019-12-31 23:59:59.998",
"2020-01-01 00:00:00",
"2020-01-01 00:00:00.002"
],
"y": [
"-1e-1",
"-1e-2",
"-1e-0"
],
"type": "scatter"
},
{
"name": "p & negative",
"xaxis": "x3",
"yaxis": "y3",
"x": [
"2019-12-31 23:59:59.998",
"2020-01-01 00:00:00",
"2020-01-01 00:00:00.002"
],
"y": [
"-1e-1",
"-1e-2",
"-1e-0"
],
"type": "scatter"
},
{
"name": "p & positive",
"xaxis": "x4",
"yaxis": "y4",
"x": [
"2019-12-31 23:59:59.998",
"2020-01-01 00:00:00",
"2020-01-01 00:00:00.002"
],
"y": [
"1e-1",
"1e-2",
"1e-0"
],
"type": "scatter"
}
],
"layout": {
"width": 800,
"height": 800,
"xaxis": {
"type": "date",
"domain": [
0,
0.45
]
},
"xaxis2": {
"type": "date",
"anchor": "y2",
"domain": [
0.6,
1
]
},
"xaxis3": {
"type": "date",
"anchor": "y3",
"domain": [
0,
0.45
]
},
"xaxis4": {
"type": "date",
"anchor": "y4",
"domain": [
0.6,
1
]
},
"yaxis": {
"nticks": 10,
"tickformat": "s",
"domain": [
0,
0.45
]
},
"yaxis2": {
"nticks": 10,
"tickformat": "s",
"anchor": "x2",
"domain": [
0,
0.45
]
},
"yaxis3": {
"nticks": 10,
"tickformat": "p",
"anchor": "x3",
"domain": [
0.6,
1
]
},
"yaxis4": {
"nticks": 10,
"tickformat": "p",
"anchor": "x4",
"domain": [
0.6,
1
]
}
}
}
108 changes: 108 additions & 0 deletions test/image/mocks/tick_percent.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
{
"data": [
{
"type": "scatter",
"y": [
1,
0,
0.5
]
},
{
"xaxis": "x2",
"yaxis": "y2",
"type": "scatter",
"y": [
1,
0,
0.5
]
},
{
"xaxis": "x3",
"yaxis": "y3",
"type": "scatter",
"y": [
1,
0,
0.5
]
},
{
"xaxis": "x4",
"yaxis": "y4",
"type": "scatter",
"y": [
1,
0,
0.5
]
}
],
"layout": {
"width": 800,
"height": 800,
"xaxis": {
"domain": [
0,
0.48
]
},
"xaxis2": {
"anchor": "y2",
"domain": [
0.52,
1
]
},
"xaxis3": {
"anchor": "y3",
"domain": [
0,
0.48
]
},
"xaxis4": {
"autorange": "reversed",
"anchor": "y4",
"domain": [
0.52,
1
]
},
"yaxis": {
"tickformat": "p",
"domain": [
0,
0.48
]
},
"yaxis2": {
"tickformat": "p",
"dtick": 0.1,
"anchor": "x2",
"domain": [
0.52,
1
]
},
"yaxis3": {
"tickformat": "p",
"dtick": 0.3,
"anchor": "x3",
"domain": [
0.52,
1
]
},
"yaxis4": {
"tickformat": "p",
"dtick": 0.05,
"anchor": "x4",
"domain": [
0,
0.48
]
}
}
}
Loading