ggplot2 icon indicating copy to clipboard operation
ggplot2 copied to clipboard

Change minor_breaks interface

Open hadley opened this issue 4 years ago • 10 comments

To pass in both limits and major breaks. This will make it much easier to create useful families of minor break functions in scales.

Fixes #3583

@thomasp85 — if you think this approach is reasonable I'll work on a NEWS bullet. I think documentation will need to happen in a separate PR because all of breaks, labels, and minor_breaks need revision to point to the reader towards the scales package.

hadley avatar Oct 27 '19 12:10 hadley

This helps a lot, but there's a major drawback: the breaks always start within the limits, so this doesn't generate any minor breaks between the minimum and first break, and the last break and the maximum.

You can see the problem with a log scale with 10 minor breaks between each major break — you should see breaks to the left of 1.

Screenshot 2019-10-27 at 9 53 21 AM

hadley avatar Oct 27 '19 13:10 hadley

But maybe I can handle that by also using the limits, and doing something more sophisticated than just using a fixed number of breaks for log scales.

hadley avatar Oct 27 '19 13:10 hadley

Just a quick note...that could partially be fixed by removing break censoring in ScaleContinuous$get_breaks(). scales::extended_breaks() often returns breaks that are outside the limits, and scales::log_breaks() I'm sure could be modified to always return breaks outside the limits. Breaks would still need to be censored before they are drawn, but this could be moved to guide_train(). There was a comment to this effect in a previous version of Scale that I removed in one of my PRs over the summer (sorry!).

paleolimbot avatar Oct 27 '19 13:10 paleolimbot

Having briefly explored the possibility of using the range, I think your suggestion would be a better approach @paleolimbot.

hadley avatar Oct 27 '19 14:10 hadley

I seem to remember trying this once, and the trick is getting manually specified labels to work right (manually specified labels have to be the same length as the within-limit breaks).

paleolimbot avatar Oct 27 '19 14:10 paleolimbot

@Hadley where are we on this?

thomasp85 avatar Dec 13 '19 09:12 thomasp85

Lets hold off on this as it needs more changes to be fully useful.

hadley avatar Dec 13 '19 13:12 hadley

minor_breaks Hello, I am new to contributing to open source, but I was trying to contribute a solution to this exact situation with minor_breaks on a log10 scale when I found this PR. It kept coming up at work and there seemed to be no convenient solution. The attached image is from a function I wrote that is supplied to minor_breaks that generalizes to any axis on a log10 scale. As you can see, it includes minor breaks past the last major breaks. I'm not exactly sure how to implement this, it doesn't seem like I can simply create a variable set to my function in scale-.r or scale-continuous.r, but with some help I may be able to help get this functionality into ggplot2.

TyStark avatar Aug 05 '21 16:08 TyStark

@Hadley is it viable to get this into the next release?

thomasp85 avatar May 11 '22 11:05 thomasp85

@thomasp85 I have no idea

hadley avatar May 11 '22 18:05 hadley

Breaks would still need to be censored before they are drawn, but this could be moved to guide_train().

I explored this for a bit, but there are plenty of places outside the guides where that approach gives issues, mostly in coords when dealing with the panel grid.

I think the change in the PR improves the current situation, and any 'full' solution would include these changes as well. Therefore, I'll suggest to merge this PR under the banner of incremental improvement and deal with the out-of-bounds breaks separately once we find a satisfactory solution to that problem.

teunbrand avatar Oct 27 '23 11:10 teunbrand

Closing this PR in favour of #5569.

teunbrand avatar Dec 08 '23 11:12 teunbrand