-
Notifications
You must be signed in to change notification settings - Fork 187
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
d3.tickIncrement? #45
Comments
Actually, it may not be necessary to deprecate or change d3.tickStep. I believe (but need to verify using a simple for loop) that in any case that d3.tickStep returns a step where |step| < 1, that 1 / step will be an integer. Even if that’s not the case, we could always Math.round it. That means that the only real change will be in code that rounds according to the tick step. So nice could look like this: var step = tickStep(start, stop, count);
if (Math.abs(step) >= 1) {
start = Math.floor(start / step) * step;
stop = Math.ceil(stop / step) * step;
} else {
step = 1 / step;
start = Math.floor(start * step) / step;
stop = Math.ceil(stop * step) / step;
} And ticks could look like this (note: abusing the count argument as a temporary): function ticks(start, stop, count) {
var step = tickStep(start, stop, count);
return Math.abs(step) >= 1 ? range(
Math.ceil(start / step) * step,
Math.floor(stop / step) * step + step / 2, // inclusive
step
) : count = 1 / step, range(
Math.ceil(start * count) / count,
(2 * Math.floor(stop * count) + 1) / (2 * count), // inclusive
step
);
} Which has the same behavior on the OP’s test case. |
Slightly cleaner? function ticks(start, stop, count) {
var step = tickStep(start, stop, count);
if (Math.abs(step) >= 1) {
start = Math.ceil(start / step) * step;
stop = (2 * Math.floor(stop / step) + 1) * step / 2; // inclusive
} else {
count = 1 / step;
start = Math.ceil(start * count) / count;
stop = (2 * Math.floor(stop * count) + 1) / (2 * count); // inclusive
}
return range(start, stop, step);
} |
Guess I’m wrong—we can’t assume that 1 / step will be an integer, at least for extreme values. We probably will need a tickIncrement method, then. https://runkit.com/57d8702e1ff21014007f12d5/58138606783f93001320ab74 |
This seems even better, where the range is always computed in integers, and then the step is applied: function ticks(start, stop, count) {
var step = tickIncrement(start, stop, count);
return step >= 0 ? range(
Math.ceil(start / step) * step,
Math.floor(stop / step) * step + step / 2, // inclusive
step
) : range(
Math.floor(start * step),
(2 * Math.ceil(stop * step) - 1) / 2, // inclusive
-1
).map(x => x / step);
} Results in: [5.8, 5.85, 5.9, 5.95, 6, 6.05, 6.1, 6.15, 6.2]! |
@mbostock Hats off, this is a very elegant solution for the task at hand (stumbled upon this while looking for the reason for having both |
Related d3/d3-scale#81, the fact that d3.tickStep can returns a floating point number can cause cascading problems in computing nice domains and ticks.
But I suspect there’s an easy fix, because the tick step is always a power of ten, optionally multiplied by 2 or 5. If the power of ten is nonnegative, the existing behavior is fine; but if it’s negative, we return the inverse tick step instead, which is likewise guaranteed to be an integer. Let’s call this the tick “increment” (or perhaps the tick “interval”). We can introduce d3.tickIncrement and deprecate d3.tickStep.
So, if the tick step is 0.05, then the tick increment would be -20. Here’s the implementation, which now requires that start ≤ stop:
Note that this is guaranteed to return an integer because powers of ten are always integer multiples of 2 and 5.
To use it to nice, the scale would do something like:
Which in the case of d3/d3-scale#81 produces the result of [5.8, 6.2]. 👍
You’d need something similar in d3.ticks (ignoring descending intervals):
Which results in [5.8, 5.85, 5.8999999999999995, 5.95…6.05, 6.1, 6.1499999999999995, 6.2], which seems reasonable.
The text was updated successfully, but these errors were encountered: