wilkelab / cowplot

cowplot: Streamlined Plot Theme and Plot Annotations for ggplot2
https://wilkelab.org/cowplot/
704 stars 84 forks source link

`get_legend` only works with default legend position, will not detect if legend is "bottom" for instance #205

Closed jkylearmstrong closed 3 months ago

jkylearmstrong commented 3 months ago
library(cowplot)
library(ggplot2)

theme_set(theme_half_open())

p1 <- ggplot(mtcars, aes(mpg, disp)) + 
  geom_line()

plot.mpg <- ggplot(mpg, aes(x = cty, y = hwy, colour = factor(cyl))) + 
  geom_point(size=2.5) +
  theme(legend.position='bottom')

legend <- get_legend(plot.mpg)

plot.mpg <- plot.mpg + theme(legend.position='none')
# Now plots are aligned vertically with the legend to the right
ggdraw(plot_grid(plot_grid(p1, plot.mpg, ncol=1, align='v'),
                 plot_grid(NULL, legend, ncol=1),
                 rel_widths=c(1, 0.2)))
sessionInfo()
R version 4.4.0 (2024-04-24 ucrt)
Platform: x86_64-w64-mingw32/x64
Running under: Windows 11 x64 (build 22631)

Matrix products: default

locale:
[1] LC_COLLATE=English_United States.utf8  LC_CTYPE=English_United States.utf8   
[3] LC_MONETARY=English_United States.utf8 LC_NUMERIC=C                          
[5] LC_TIME=English_United States.utf8    

time zone: America/New_York
tzcode source: internal

attached base packages:
[1] stats     graphics  grDevices datasets  utils     methods   base     

other attached packages:
[1] ggplot2_3.5.1 cowplot_1.1.3

loaded via a namespace (and not attached):
 [1] labeling_0.4.3    utf8_1.2.4        R6_2.5.1          tidyselect_1.2.1 
 [5] farver_2.1.2      magrittr_2.0.3    gtable_0.3.5      glue_1.7.0       
 [9] tibble_3.2.1      pkgconfig_2.0.3   generics_0.1.3    dplyr_1.1.4      
[13] lifecycle_1.0.4   cli_3.6.2         fansi_1.0.6       scales_1.3.0     
[17] grid_4.4.0        vctrs_0.6.5       withr_3.0.0       renv_1.0.7       
[21] compiler_4.4.0    rstudioapi_0.16.0 tools_4.4.0       pillar_1.9.0     
[25] munsell_0.5.1     colorspace_2.1-0  rlang_1.1.4  

image

clauswilke commented 3 months ago

This is a duplicate of #202. That issue also contains a suggested alternative implementation by Teun van den Brand that I think is the right way forward. If you want to prepare a PR based on that you're welcome to do so.

jkylearmstrong commented 3 months ago

@clauswilke my apologies, I should have checked to see if someone else had already commented on the error.

From the discussion, it seems like the fix in https://github.com/wilkelab/cowplot/issues/202 allows the user to pass along the expected location which is also reasonable.

It still might be worthwhile to consider filtering out the zeroGrubs from get_plot_component when the user only expects one return, at least then the return object will always contain a non-zero grub if the expected behavior is to return a zeroGrub if all are zero, then you could modify my proposed solution https://github.com/wilkelab/cowplot/pull/206 to account for that.

Either way, thank you for this package, it's handy, just trying to see if I could help contribute and make it better! :)

clauswilke commented 3 months ago

That's fine, you're welcome to merge your ideas and Teun's ideas in your PR. I haven't thought about this function much recently but I agree it should be fixed.