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

Ticks of niced linear scales do not always cover the domain #209

Closed
domoritz opened this issue May 20, 2020 · 9 comments · Fixed by #210
Closed

Ticks of niced linear scales do not always cover the domain #209

domoritz opened this issue May 20, 2020 · 9 comments · Fixed by #210

Comments

@domoritz
Copy link
Contributor

domoritz commented May 20, 2020

> d3
  .scaleLinear()
  .domain([70, 210])
  .nice(2)
  .ticks(2)
[0,200]

https://github.com/d3/d3-scale#continuous_nice says that "An optional tick count argument allows greater control over the step size used to extend the bounds, guaranteeing that the returned ticks will exactly cover the domain." This is not true here and the scale domain ([0, 300]) is not covered by the ticks [0,200].

This issue looks similar to #9

@mbostock
Copy link
Member

So, the correct answer here is a domain of [0, 500] and ticks [0, 500]?

I’m not really sure how to implement the documented guarantee, but I’m open to suggestions.

@domoritz
Copy link
Contributor Author

I think I would want a domain of [0,400] and ticks [0,200,400], which is what I am getting when I use a domain of [60, 210] (just a slightly different start).

d3
  .scaleLinear()
  .domain([60, 210])
  .nice(2)
  .ticks(2)

@mbostock
Copy link
Member

Ah, yes, that’s slightly better. And indeed that’s what you get if you nice twice:

> d3
  .scaleLinear()
  .domain([70, 210])
  .nice(2)
  .nice(2)
  .ticks(2)
[0, 200, 400]

@domoritz
Copy link
Contributor Author

By the way, if we run the iteration in nice three times, we get the "right" answer. I didn't quite get what you meant by an iterative solution in #9 (comment).

Maybe we need to run the iteration to a fixpoint?

@mbostock
Copy link
Member

I was just going to make the same suggestion.

@domoritz
Copy link
Contributor Author

domoritz commented May 20, 2020

I could take a stab at it but I'm not exactly sure what the fixpoint would be for. Do you have cycles to implement a fix?

I did write some test code already.

tape("linear.ticks(X) spans linear.nice().domain()", function(test) {
  function check(domain, count) {
    var s = scale.scaleLinear().domain(domain).nice(count);
    var ticks = s.ticks(count);
    test.deepEqual([ticks[0], ticks[ticks.length - 1]], s.domain());
  }
  
  check([1, 9], 2);
  check([1, 9], 3);
  check([1, 9], 4);

  check([8, 9], 2);
  check([8, 9], 3);
  check([8, 9], 4);

  check([1, 21], 2);
  check([2, 21], 2);
  check([3, 21], 2);
  check([4, 21], 2);
  check([5, 21], 2);
  check([6, 21], 2);
  check([7, 21], 2);
  check([8, 21], 2);
  check([9, 21], 2);
  check([10, 21], 2);

  test.end();
})

@domoritz
Copy link
Contributor Author

The tick calculation feels somewhat similar to @jheer's binning code in https://github.com/vega/vega/blob/master/packages/vega-statistics/src/bin.js.

@mbostock
Copy link
Member

I think this will do it:

function nice(start, stop, count) {
  let prestep;
  while (true) {
    const step = tickIncrement(start, stop, count);
    if (step === prestep) {
      return step;
    } else if (step > 0) {
      start = Math.floor(start / step) * step;
      stop = Math.ceil(stop / step) * step;
    } else if (step < 0) {
      start = Math.ceil(start * step) / step;
      stop = Math.floor(stop * step) / step;
    } else {
      return NaN;
    }
    prestep = step;
  }
}

https://observablehq.com/d/7acf042a83b43c16

@domoritz
Copy link
Contributor Author

I see. So you are running the fixpoint calculation until the step size has settled. Sounds good to me.

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

Successfully merging a pull request may close this issue.

2 participants