breaks argument in stat_contour() accepts function.

[eliocamp]

Extended the breaks argument of stat_contour() to accept a function (similar to breaks in scale_x_continuous()).

Also, created breaks_default() which is the default function for creating breaks. The rational being to give the user easy access to the default behaviour so they can use it as the starting point for a new breaks function or small modifications. For example:

library(ggplot2)

v <- ggplot(faithfuld, aes(waiting, eruptions, z = density))
v + geom_contour(binwidth = 0.001)

my_breaks <- function(range) {
  b <- breaks_default(binwidth = 0.001, NULL)(range)
  b[b != 0.002]
}
v + geom_contour(breaks = my_breaks)

Also added documentation to bins and binwidth.

[eliocamp]

This is workable as is, but I'm still arguing with myself whether to pass only the range of the data or also bins and binwidth to the function in breaks.

[hadley]

I think it should pass binwidth. That would effectively make breaks = fullseq the default behaviour (but I don't think you should actually make that the default value)

[eliocamp]

Using fullseq as de default wouldn't break backwards compatibility in some cases? Right now the default when no parameter is passed is pretty(range, 10).

[hadley]

I'm not saying you should change it; I'm saying that it would match the existing internal API.

[hadley]

Can you please preserve the existing indentation?

[hadley]

I think we've gone off the rails a bit here because I wasn't clear exactly what I was thinking. I'd prefer not to make such a big change and instead of breaks is a function, call it like fullseq with the range of the data and the binwidth.

[eliocamp]

Sorry about the indent. 😅

I hope this is better. I still think the previous default behaviour (when all arguments are NULL) doesn't translate easily to breaks() only accepting range and binwidth. In this version, if no bins or binwidth is specified, I calculate binwidth from the result of pretty().

Are you open to removing pretty() completely and just set bins to 10 as default and calculate binwidth from diff(data$z)/bins? It's less convoluted and consistent. It also would fix the (arguably) bug that geom_contour(bins = 10) has a different result that geom_contour() even thought both have the same number of bins.

[thomasp85]

@clauswilke whenever you have the time, can you give this PR a look and see if it should be put in line for the next feature release (since you "own" contouring around here😄)

[clauswilke]

Yes. I see this is a really old PR though, and the underlying code has changed a lot.

@eliocamp Are you willing to take this back up and make it workable with the current code base?

[thomasp85]

Hi @eliocamp

This PR was closed as part of our default branch renaming efforts. Since the PR appeared to be stalled I'll keep it closed since it requires a bit of work to resurrect it.

If you feel like taking a second stab at this, please open a new PR (and make sure to update your ggplot2 fork to point to the new 'main' branch (using usethis::git_default_branch_rediscover())