Implement n_breaks in continuous_scale

[thomasp85]

This is following thoughts and discussion in #3096 and implement an n_breaks argument in continuous_scale() to allow guiding the number of major breaks that gets created. Before you'd had to make the breaks manually if you wanted to have a different number of breaks (or construct your own trans object by hand)...

The implementation can feel a bit hacky as it modifies the parent environment of the breaks method of the trans object. I make sure to reset the parent environment on exit so there should be no side effects, but a more elegant approach is if scales allowed dynamic setting of n in trans$breaks()

Anyway, consider this PR as discussion ground for the feature and we can figure out the best implementation later if we want it...

example use:

library(ggplot2)

ggplot(mtcars) + 
  geom_point(aes(mpg, disp))

ggplot(mtcars) + 
  geom_point(aes(mpg, disp)) + 
  scale_x_continuous(n_breaks = 10)

^{Created on 2019-01-25 by the reprex package (v0.2.1)}

[hadley]

Can't we implement this by setting breaks to a function?

[thomasp85]

Indeed, but I think having a direct argument accepting an integer will make users actually use it... having to construct a function that wraps something from the labelling package seems a high bar for such a simple need (IMO)

[thomasp85]

Or do you mean that we should do this under the hood during construction of the scale?

[thomasp85]

I think it is going to be hard to implement under the hood as functions generally because different transformations have different breaks functions, so we'd have to reverse engineer these... this is the reason why I've currently accepted the approach implemented here

[hadley]

If you think this is really important, then I think you need to start by refactoring the existing breaks code to make it easier to pass n_breaks along to the right place. I don't think hacking into the environment like this is a good idea.

[thomasp85]

I don’t think that it is super important, but it needs to be figured out for scale_binned anyway where it’ll be an important argument. I’d much rather fix this in scales, so if you’d accept a fix there I’d happily provide one

[hadley]

Yeah, I'd happily accept a fix in scales

[hadley]

What's up with the big diff here?

[thomasp85]

You must be looking at the code from before I merged master in? scale-.r was reorganised while this PR was developed

[lock]

This old issue has been automatically locked. If you believe you have found a related problem, please file a new issue (with reprex) and link to this issue. https://reprex.tidyverse.org/

[lock]

This old issue has been automatically locked. If you believe you have found a related problem, please file a new issue (with reprex) and link to this issue. https://reprex.tidyverse.org/