fix a bug in ScaleBinned$get_breaks()

[microly]

this pull request try to fix the bug in issue #3574 .

I set n.breaks as the second parameter of breaks function.
if the n.breaks is not set by users, it will set to the default value 5, which is the same default as trans objects.

n.breaks can be set by end users.
So, after this fix, users can get meaningful breaks by set breaks as a function and get meaningful breaks using the values of limits and n.breaks.

[thomasp85]

I think we'd need to check if the breaks function support two arguments before calling it with n.breaks, as the assumptions up until now has been that a breaks function only take a single argument

[microly]

I think the breaks function with two arguments can work well.
The code block below is a simple example.

The brk function can be also used in other scales whose breaks function only need a single argument, because n.breaks has a default value.

df <- data.frame(
    x = runif(100),
    y = runif(100),
    z = rnorm(100)
)

brk <- function(limits, n.breaks = NULL) {
    n.breaks <- n.breaks %||% 5
    breaks <- seq(limits[1], limits[2], length.out = n.breaks + 2)
    breaks <- breaks[-c(1, length(breaks))]
}

library(ggplot2)

ggplot(df, aes(x, y)) +
    geom_point(aes(colour = z)) +
    scale_colour_stepsn(colours = terrain.colors(10), breaks = brk)

ggplot(df, aes(x, y)) + geom_point() + scale_x_continuous(breaks = brk)

[thomasp85]

What I mean is that your change will mean that if you pass in a breaks function that only accepts a single argument it will fail. This runs counter to the expectations of the breaks function

[microly]

oh, I know what you mean now~

maybe, we can handle it in some way like:

ggplot2/R/scale-.r

Lines 955 to 956 in 115c396

	if (!is.null(self$n.breaks) && "n" %in% names(formals(self$trans$breaks))) {
	breaks <- self$trans$breaks(limits, n = self$n.breaks)

[thomasp85]

yeah, something like that

[microly]

OK, thanks for your reply!
I learn very much from your codes and packages, thanks!

How can I finish the work? should I open a new pull request?

[clauswilke]

@microly Just make additional changes to the code, commit, push, and the PR will update. Don't open a new PR.

[microly]

@clauswilke
Thanks for your reply.
I am mot familiar with github, but I will try my best!

By the way, cowplot is one of the most important package in my daily work, thanks for your great
work!

[microly]

@thomasp85
@clauswilke
I have made an update, please check it, thanks!

[thomasp85]

Thanks!

[microly]

thanks for your work!

[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/