thomasp85 / patchwork

The Composer of ggplots
https://patchwork.data-imaginist.com
Other
2.46k stars 162 forks source link

Shared titles and axes #337

Closed teunbrand closed 9 months ago

teunbrand commented 11 months ago

This PR aims to fix #150.

Briefly, it adds options to plot_layout() that allows the user to:

  1. Collect titles, i.e. deduplicate them and merge them
  2. Deduplicate axes

What can and cannot be merged is mostly determined by a homebrew 2D run-length encoding of a simplified layout.

An example using identical plots to showcase the sharing. Notice the following:

library(ggplot2)
devtools::load_all("~/packages/patchwork/")
#> ℹ Loading patchwork

# loading dev patchwork overrides theme
theme_set(theme_grey())

p1 <- ggplot(mtcars) +
  aes(x = cyl, y = disp) +
  geom_point() +
  guides(x.sec = "axis", y.sec = "axis")

layout <- plot_layout(
  design = c("1234\n5267"),
  collect_titles = "both",
  dedup_axes = "both"
)

p1 + p1 + p1 + p1 + p1 + p1 + p1 + layout

Another example showing not-merge behaviour for the last plot. Notice the following:

p2 <- p1 + theme(axis.text = element_text(colour = "red")) + 
  labs(x = "foo", y = "bar")

p1 + p1 + p1 + p1 + p1 + p1 + p2 + layout

Created on 2023-10-23 with reprex v2.0.2

Why is this PR still WIP?

p1 <- p1 + scale_y_continuous(labels = \(x) paste0(x, " but with longer stuff")) +
  guides(x.sec = "none", y.sec = "none")

p1 + p1 + p1 + p2 + plot_layout(2, 2, dedup_axes = "y")

thomasp85 commented 11 months ago
teunbrand commented 11 months ago

Thanks for your thoughts Thomas!

This suggests adding a axes and axis_titles argument to plot_layout() that can take "keep" and "collect" as values

Is there a need to discriminate the x- or y-direction and if so, how would you like this to be exposed? We could add x_axes = NULL and y_axes = NULL arguments, where x_axes = x_axes %||% axes or something. Or should the options be "keep", "collect", "collect_x" and "collect_y"?

thomasp85 commented 11 months ago

If we should discriminate I think the last suggestion is the best since we already have committed to string values

teunbrand commented 11 months ago

Latest updates:

I think this PR is now in a good place for review.

library(ggplot2)
devtools::load_all("~/packages/patchwork/")
#> ℹ Loading patchwork

theme_set(theme_grey())

p1 <- ggplot(mtcars) +
  aes(x = cyl, y = disp) +
  geom_point()

p2 <- p1 + 
  theme(axis.text = element_text(colour = "red")) + 
  labs(x = "foo", y = "bar") 

p1 <- p1 + scale_y_continuous(labels = \(x) paste0("a long label indicating ", x))

p1 + p1 + p1 + p2 + plot_layout(2, 2, axes = "collect", axis_titles = "collect")

Created on 2023-11-02 with reprex v2.0.2

thomasp85 commented 9 months ago

Thank you!

bwiernik commented 9 months ago

Amazing!

pabloabur commented 3 months ago

Hi,

It seems like this doesn't do anything when we use | and / instead of plot_layout definition. Is this intended?

p1 <- ggplot(mtcars) +
  aes(x = cyl, y = disp) +
  geom_point()

p2 <- p1 + 
  theme(axis.text = element_text(colour = "red")) + 
  labs(x = "foo", y = "bar") 

p1 <- p1 + scale_y_continuous(labels = \(x) paste0("a long label indicating ", x))

((p1 | p1) / (p1 | p2)) + plot_layout(axes = "collect", axis_titles = "collect")

test

teunbrand commented 3 months ago

Is this intended?

Yes and no. The merging mechanism don't work over multiple nesting levels, which is what you've induced with your layout code. We had accepted this limitation, so while we agree that it doesn't work out ideally in your case, it generally does what we had intended. The solution to the problem would be to use an unnested layout design.

pabloabur commented 3 months ago

Fair enough. Is there a way to raise a warning in this situation? Or alternatively mention it on the documentation? I could make a pull request (although the former seems to be complicated)