Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

scale_fill_manual(drop = FALSE) results in missing key aesthetics #5728

Closed
kevinwolz opened this issue Feb 28, 2024 · 14 comments
Closed

scale_fill_manual(drop = FALSE) results in missing key aesthetics #5728

kevinwolz opened this issue Feb 28, 2024 · 14 comments

Comments

@kevinwolz
Copy link

This is an issue that popped up for me when updated from version 3.4.4 to 3.5.0.

library(sf)
library(dplyr)
library(ggplot2)

fname <- 
nc <- system.file("shape/nc.shp", package = "sf") |>
  read_sf() |>
  mutate(AREA_F = cut(AREA, 5)) |>
  filter(AREA < 0.122)

VERSION 3.4.4

Both of these functioned as I would expect.
drop = TRUE

ggplot() +
  geom_sf(data = nc, aes(fill = AREA_F)) +
  scale_fill_manual(values = rainbow(5), drop = TRUE)

image

drop = FALSE

ggplot() +
  geom_sf(data = nc, aes(fill = AREA_F)) +
  scale_fill_manual(values = rainbow(5), drop = FALSE)

image

VERSION 3.5.0

drop = TRUE is the same, as expected.

ggplot() +
  geom_sf(data = nc, aes(fill = AREA_F)) +
  scale_fill_manual(values = rainbow(5), drop = TRUE)

image

drop = FALSE adds the missing levels to the legend, but the keys are empty.

ggplot() +
  geom_sf(data = nc, aes(fill = AREA_F)) +
  scale_fill_manual(values = rainbow(5), drop = FALSE)

image

@teunbrand
Copy link
Collaborator

teunbrand commented Feb 28, 2024

Hi there, thanks for the report! This is indended behaviour from the 3.5.0 update: if your data doesn't contain the level, the key isn't rendered for that layer by default. To render the keys for non-existing data, you should set geom_sf(..., show.legend = TRUE). This is documented in the news:

ggplot2/NEWS.md

Lines 36 to 38 in bc3e401

* By default, `guide_legend()` now only draws a key glyph for a layer when
the value is in the layer's data. To revert to the old behaviour, you
can still set `show.legend = c({aesthetic} = TRUE)` (@teunbrand, #3648).

And communicated in the release blog: https://www.tidyverse.org/blog/2024/02/ggplot2-3-5-0-legends/#awareness

There is a separate issue for making this clearer in the documentation of scales, see #5715.
We can leave this issue open for others to find for a while.

@kevinwolz
Copy link
Author

kevinwolz commented Mar 8, 2024

@teunbrand thanks for the help. I agree that improving the documentation would help.

I have come across some other unexpected (for me at least) behavior that I wanted to ask about. This may be an issue with the package tidyterra, but it also potentially warrants additional documentation so I figured I would ask it here as well.

library(sf)
library(terra)
library(tidyterra)
library(ggplot2)

nc <- system.file("shape/nc.shp", package = "sf") |>
  read_sf()

# Generate sf points
sf_points <- nc |>
  st_centroid() |>
  mutate(shp = factor(rep(c(1, 2), length.out = nrow(nc))))

# Generate categorical raster
terra_raster <- nc |>
  vect() |>
  rasterize(rast(nc), background = 2)

The following plot works as expected. Some points on top of a raster, with just the legend for the points.

ggplot() +
  tidyterra::geom_spatraster(data = terra_raster, show.legend = FALSE) +
  geom_sf(data = sf_points, aes(shape = shp))

image

When I set show.legend = TRUE within tidyterra::geom_spatraster, I get the raster legend (as expected), but then suddenly the key background in the shape legend is dark grey. It is surprising to me that changing a parameter in tidyterra::geom_spatraster would impact the legend of a totally different layer.

ggplot() +
  tidyterra::geom_spatraster(data = terra_raster, show.legend = TRUE) +
  geom_sf(data = sf_points, aes(shape = shp))

image

My sessionInfo():

R version 4.3.2 (2023-10-31)
Platform: aarch64-apple-darwin20 (64-bit)
Running under: macOS Monterey 12.4

Matrix products: default
BLAS:   /System/Library/Frameworks/Accelerate.framework/Versions/A/Frameworks/vecLib.framework/Versions/A/libBLAS.dylib 
LAPACK: /Library/Frameworks/R.framework/Versions/4.3-arm64/Resources/lib/libRlapack.dylib;  LAPACK version 3.11.0

locale:
[1] en_US.UTF-8/en_US.UTF-8/en_US.UTF-8/C/en_US.UTF-8/en_US.UTF-8

time zone: America/Chicago
tzcode source: internal

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

other attached packages:
[1] ggplot2_3.5.0   tidyterra_0.5.2 terra_1.7-71    sf_1.0-15      

loaded via a namespace (and not attached):
 [1] vctrs_0.6.5        cli_3.6.2          rlang_1.1.3        DBI_1.2.2         
 [5] KernSmooth_2.23-22 purrr_1.0.2        generics_0.1.3     data.table_1.15.0 
 [9] labeling_0.4.3     glue_1.7.0         colorspace_2.1-0   e1071_1.7-14      
[13] scales_1.3.0       fansi_1.0.6        grid_4.3.2         munsell_0.5.0     
[17] classInt_0.4-10    tibble_3.2.1       lifecycle_1.0.4    compiler_4.3.2    
[21] dplyr_1.1.4        codetools_0.2-19   Rcpp_1.0.12        pkgconfig_2.0.3   
[25] tidyr_1.3.1        rstudioapi_0.15.0  farver_2.1.1       wk_0.9.1          
[29] R6_2.5.1           class_7.3-22       tidyselect_1.2.0   utf8_1.2.4        
[33] pillar_1.9.0       magrittr_2.0.3     withr_3.0.0        tools_4.3.2       
[37] proxy_0.4-27       gtable_0.3.4       s2_1.1.6           units_0.8-5   

@teunbrand
Copy link
Collaborator

teunbrand commented Mar 9, 2024

Setting show.legend = TRUE displays the terra layer on every legend. If you want to tweak on which legend to display the layer, you can set show.legend = c(fill = TRUE, shape = FALSE).
Thinking about it now, setting a named TRUE should probably imply a FALSE for any others, but that isn't implemented.

@njudd
Copy link

njudd commented Apr 7, 2024

This is a very unintuitive change, I feel like show.legend = TRUE should default drop = FALSE or visa versa

@teunbrand
Copy link
Collaborator

This change is to remove the need for setting very complicated and tailored guide_legend(override.aes) argument just to have legend visuals match the plot. In theory, we could let drop = FALSE propagate layers where show.legend = NA, which would be straightforward if the plot has a single layer. However, this would then apply to all layers regardless of the level occurs in the data or not, thereby losing control.

@ECarnell
Copy link

ECarnell commented Apr 17, 2024

I agree with njudd, this seems very counterintuitive. So drop = F essentially becomes redundant?

Original code

df <- data.frame(x = runif(n = 50, min = 0, max = 10),
y = runif(n = 50, min = 0, max = 10),
z = runif(n = 50, min = 0, max = 4))

df$cat <- cut(df$z,
breaks = c(0,1,2,3,4,6,8),
labels = c("0 - 1","> 1 - 2","> 2 - 3",
"> 3 - 4","> 4 - 6","> 6 - 8"))

p <- ggplot()

p <- p + geom_point(data = df, aes(x = x, y = y, colour = cat), size = 5)

p <- p + scale_color_manual(values = c("#38d1ff","#fffc36","#ffa536","#ff2e2e","#f314ff","#9514ff","#C1CDCD"),
limits = names(cols),
drop = F,
bquote(mu*g~NH[3]~m^-3))

p

image

Removing drop = F (now redundant) and adding show.legend = T produces:
image

@teunbrand
Copy link
Collaborator

teunbrand commented Apr 17, 2024

So drop = F essentially becomes redundant?

You still need it to include unobserved levels.

@ECarnell
Copy link

You still need it to include unobserved levels

Would it not make sense to also include the colours associated with those unobserved levels? (especially for consistency with other plots where the levels are observed)

@teunbrand
Copy link
Collaborator

Yes and no. Yes it makes intuitive sense, but no, it destroys the ability to control this at the geom level.

@ECarnell
Copy link

I agree controlling this at the geom level makes sense, but currently the behaviour of drop in scale_XXX_manual is counterintuitive. I think the drop argument should override show.legend (especially when breaks/labels are set explicitly)

@thomasp85
Copy link
Member

I can emphasise with the annoyance of this change for your particular use cases, but making the requested change would in practice make it impossible to have show.legend = NA and drop = TRUE at the same time (something that other will find useful), whereas your issues are solvable by setting the right arguments. In that case, not limiting other users win

@ECarnell
Copy link

Should the help/documentation of scale_XXX_manual be updated to reference this change in behaviour? Noting that the behaviour of drop will be depended on show.legend

@teunbrand
Copy link
Collaborator

The documentation was adjusted to reflect this change better in #5732 and will be included in the next release.

@teunbrand
Copy link
Collaborator

Alright with 3.5.1 now released and we don't have the intent to change this, I believe it is time to close this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants