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.

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

Support decimal stepSize #5786

Merged
merged 1 commit into from Nov 12, 2018
Merged

Support decimal stepSize #5786

merged 1 commit into from Nov 12, 2018

Conversation

nagix
Copy link
Contributor

@nagix nagix commented Oct 24, 2018

This PR adds support for decimal stepSize for linear scales. In the current code,

  • if stepSize >= 1, ticks will be rounded to integers
  • if stepSize < 1, ticks will have the appropriate decimal precision
  • If stepSize is not specified, the tick interval will be set to a 'nice' number, so there is no rounding issue

In this PR, the decimal precision is calculated based on stepSize if specified, or a generated 'nice' number or ticks.precision (only if stepSize is not specified).

Fixes #5392
Fixes #5579

// If the user specified a precision, round to that number of decimal places
factor = Math.pow(10, precision);
spacing = Math.ceil(spacing * factor) / factor;
}
}
var niceMin = Math.floor(dataRange.min / spacing) * spacing;
var niceMax = Math.ceil(dataRange.max / spacing) * spacing;
// If a precision is not speified, calculate factor based on spacing
Copy link
Contributor

Choose a reason for hiding this comment

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

speified -> specified

@simonbrunel
Copy link
Member

Maybe I didn't understand the issue / feature behind both options: I'm a bit worried about mixing both configs and have stepSize also controlling the precision when < 1 (overriding precision, which doesn't seem correct). Shouldn't we simply apply precision every time, not only when stepSize is undefined?

if stepSize < 1, ticks will have the appropriate decimal precision

I think we should support that case: stepSize: 0.524, precision: 2 (expecting 0, 1.05, 1.57, etc.)

if stepSize >= 1, ticks will be rounded to integers

What about: stepSize: 1.524, precision: 2? (expecting 0, 1.52, 3.05, etc.)

@nagix
Copy link
Contributor Author

nagix commented Oct 25, 2018

When stepSize is specified, we don’t need precision at all because stepSize can controll the precision. On the other hand, when stepSize is not specified but precision is specified, the tick interval is aligned with precision. To make things consistent, the same rule should be applied when both stepSize and precision are specified, but this may cause confusion. eg. stepSize: 0.524, precision: 2 results in 0, 0.52, 1.04, 1.56.

In that sense, it seems natual to me that precision is effective only when stepSize is not specified.

@nagix
Copy link
Contributor Author

nagix commented Oct 25, 2018

Or, should the auto-generated tick interval be completely independent from the precision? Should precision be only applied to the generated tick values?

In that case, there may be another confusion. eg. precision: 0 and the auto-generated tick interval: 1.5 results in 0, 2, 3, 5, 6.

@simonbrunel
Copy link
Member

I understand, I thought that precision were used only on the displayed tick value (your previous comment), that's why I didn't understand why it's ignored when stepSize is not defined.

@nagix
Copy link
Contributor Author

nagix commented Oct 28, 2018

Thanks @benmccann, I have added a jsdoc and fixed a typo.

@simonbrunel simonbrunel added this to the Version 2.8 milestone Nov 10, 2018
Copy link
Member

@simonbrunel simonbrunel left a comment

Choose a reason for hiding this comment

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

... but this may cause confusion. eg. stepSize: 0.524, precision: 2 results in 0, 0.52, 1.04, 1.56

I think the confusion comes from the fact that the precision applies to two concepts: the tick generation and the displayed format. Initially introduced to fix #4103, it's definitely not to control the displayed format, so you right.

Or, should the auto-generated tick interval be completely independent from the precision? Should precision be only applied to the generated tick values?

I think it's possible to override the display format with scale.ticks.callback but that's not really convenient.

Thanks @nagix

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants