WIP: scale-like objects in panel_params that can be used to train guides

[paleolimbot]

In order for position guides to act like other guides (#3322), there needs to be a scale-like object for the position guides to be trained on. This PR is one experiment on how to do this.

Currently, the panel_params are the only valid representation of the position scales that is available at draw time, as scales are not expanded in place, but temporarily expanded by the Coord as part of the Coord$setup_panel_params(). The panel_params for most coordinate systems is based on Scale$break_info(), which is only ever used in coordinate systems. The panel_params serve as the source of information for the Coord transformation functions, range functions, axis guides, and panel guides (the panel grid/grill). Some layers (like geom_dotplot(), annotation_logticks(), and ggspatial::annotation_scale()) also use the x.range and y.range elements of the panel_params.

Expanded position scales used by coordinate systems serve a different purpose than scales used for training, and at first I tried coercing the existing scales to serve as both but even though it sort of works, it's messy and causes at least two tests to fail.

This PR introduces an internal class, ViewScale, that serves as the definitive source of truth for coordinate systems (and potentially layers) regarding the expanded position scale. To demonstrate how it would be used, I refactored CoordCartesian to use view scales instead of a composite of Scale$break_info() list()s. Ideally, the break_info() function should be moved to the ViewScale() constructor to separate the concerns of Scale objects (transforming, censoring, training, and calculating limits/breaks/labels) from the concerns of the Coord/Geom (transforming position aesthetics from transformed data space to coordinate space) from the concerns of the guide (what are the break values, break labels, and break positions in coordinate space). Only CoordCartesian has a 1-to-1 mapping between a ViewScale and a future guide_axis(), but all the Coords need information about the expanded x and y scales and could potentially be refactored to use them.

The main questions I have are:

• Is this a reasonable way to write a scale-like object with which to train a future guide_axis()
• Should CoordCartesian use ViewScales for anything except guides?
• If so, should other coordinate systems use ViewScales?

[clauswilke]

I think this is going in the right direction. It's often bugged me that there is no authoritative source of information of the exact axis range in a plot, and it seems this PR would fix that. I find the function names a bit confusing, though. E.g., what does view_scales_from_scale() do? It might help if you could add some comments that explain what the various pieces do. That may also provide better guidance about whether the names are appropriately chosen or not.

Would this allow different panels to have different scales? As an example, we don't currently (I believe) have a faceting system that can do something like the plot below, but after attending the grammar of graphics session at SDSS 2019 I believe this is a major limitation of ggplot2 and should be supported in the future. It would require that we can somehow access and style the scales for each panel.

library(ggplot2)
library(patchwork)

p1 <- ggplot(mtcars, aes(disp, mpg)) + geom_point()
p2 <- ggplot(mtcars, aes(wt, mpg)) + geom_point()
p1 + p2

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

[hadley]

Seems like a good start to me. I think it's worth pushing this PR out a bit more just to make sure the new code is as clean as possible. How hard would it be to make view_scale() a method of the scale, so you don't need to quite so much munging of the output from break_info()?

[hadley]

Maybe also make draw_view_grid()?

[paleolimbot]

@clauswilke you're right about the function names...I think Hadley's suggestion that the function live in Scale will fix that. I think the facetting along variables is a separate issue (I'll open one) but this will definitely make that code look prettier.

[hadley]

What are all the changes here?

[paleolimbot]

These are changes that were required to keep the view scale constructor short. There are a few that were required to get previous version of the code to work that I can either take out or put into a separate pull request. Finally, there were a lot of comments and some documentation that were straight up lying, which I redocumented since I had to figure out what all these methods did to write everything else. I think this should probably be a separate pull request.

[hadley]

I think the code is a bit easier to follow if the parent class is defined first.

[paleolimbot]

As in, view_scale() should go after ViewScale <- ggproto(...) ?

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