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

Error when combining scale_color_manual and guide_legend (override.aes) in v3.3.4 #4511

Closed
tungttnguyen opened this issue Jun 14, 2021 · 7 comments
Labels
bug an unexpected problem or unintended behavior
Milestone

Comments

@tungttnguyen
Copy link

tungttnguyen commented Jun 14, 2021

With the reprex below, I got the following error when changing the alpha and size values using guide_legend (override.aes). Note that the current CRAN version (v3.3.3) worked without any error.

Error in `[[<-.data.frame`(`*tmp*`, i, value = c(0.6, 0.6)) : 
  replacement has 2 rows, data has 3

Reprex (data taken from this issue)

library(ggplot2)
packageVersion("ggplot2")
#> [1] '3.3.3.9000'

groups <- LETTERS[1:3]

df <- data.frame(
  x = rep(1:10, 3),
  y = rnorm(30),
  group = rep(groups, each = 10)
)

Set up manual color scale

my_color <- setNames(c("blue", "green", "red"), unique(df$group))
my_color
#>       A       B       C 
#>  "blue" "green"   "red"

Filter out group "A"

df1 <- df[df$group != "A", ]
df1
#>     x             y group
#> 11  1  0.7970884438     B
#> 12  2  1.0696606257     B
#> 13  3 -0.4524050451     B
#> 14  4  0.1683655197     B
#> 15  5  0.0723548494     B
#> 16  6  0.2621733369     B
#> 17  7 -0.6167289461     B
#> 18  8  0.3669677647     B
#> 19  9 -0.6197559708     B
#> 20 10 -0.6741798000     B
#> 21  1 -0.9238588784     C
#> 22  2  1.0908007314     C
#> 23  3 -0.8173714908     C
#> 24  4  0.3555677658     C
#> 25  5  0.0862835605     C
#> 26  6 -0.1650330989     C
#> 27  7 -0.6288903181     C
#> 28  8 -0.2195618380     C
#> 29  9  1.1759632572     C
#> 30 10  0.0005726088     C
n_group <- length(unique(df1$group))
n_group
#> [1] 2

p1 <- ggplot(df1, aes(x, y, colour = group)) +
  geom_point(size = 3)
p1

Legend for group "A" shows up even though we don't have it anymore (this wasn't the problem with v3.3.3 CRAN)

p1 +
  scale_colour_manual(
    "",
    values = my_color
  )

p1 +
  scale_colour_manual(
    "",
    values = my_color
  ) +
  guides(color = guide_legend(title = "",
                              override.aes = list(alpha  = rep(0.6, n_group),
                                                  size   = rep(6,   n_group)),
                              order = 1),
         stroke = "none",
         shape  = "none")
#> Error in `[[<-.data.frame`(`*tmp*`, i, value = c(0.6, 0.6)): replacement has 2 rows, data has 3

Created on 2021-06-14 by the reprex package (v2.0.0)

@madelhorius
Copy link

This issue also occurs in the newest release version 3.3.4. As described, all colors in my_colors will appear in the legend, even if the respective values are not present in the data plotted. This was not the case in version 3.3.3. Ist this an intended behaviour?

@aosmith16
Copy link

This change may have been added when fixing issue 3451 to allow the values vector to have fewer elements than the data.

This was done in pull request #4471. There I can see the following lines added to manual_scale():

  if (is.null(limits)) {
    limits <- names(values)
  }

Changing the limits to the value names in this scenario, where the vector of names contains more values than the plotting dataset, could cause this new behavior. This approach would appear to potentially bypass the scale_manual() default drop = TRUE. The user would then need to manually set the limits to only the levels they want in the legend.

I haven't thought of a straightforward fix; maybe there is a way to set the limits to the names of only the values that are present in the dataset?

In terms of the original issue with override.aes, note if you want to set the overall look of all key symbols in the legend you can use constants instead of repeating values via rep():

override.aes = list(alpha  = 0.6,
                    size   = 6)

@clauswilke clauswilke added the bug an unexpected problem or unintended behavior label Jun 22, 2021
@clauswilke clauswilke added this to the ggplot2 3.4.0 milestone Jun 22, 2021
@clauswilke
Copy link
Member

@aosmith16 Thanks for tracking this down. Yes, this is the problem. Not sure how to fix it either off the top of my head.

@teunbrand
Copy link
Collaborator

I've found setting limits = force to be a convenient workaround to the issue of the 'extra' legend items.

@tungttnguyen tungttnguyen changed the title Error when combining scale_color_manual and guide_legend (override.aes) in v3.3.4-rc Error when combining scale_color_manual and guide_legend (override.aes) in v3.3.4 Jun 22, 2021
david-barnett added a commit to david-barnett/microViz that referenced this issue Jun 25, 2021
banfai added a commit to banfai/ggplot2 that referenced this issue Jul 7, 2021
* issue introduced in tidyverse#4471
* potential solution for tidyverse#4534 and tidyverse#4511
* add extra tests
banfai added a commit to banfai/ggplot2 that referenced this issue Jul 7, 2021
* issue introduced in tidyverse#4471
* potential solution for tidyverse#4534 and tidyverse#4511
* add extra tests
@pmenzel
Copy link

pmenzel commented Jul 28, 2021

Oh, that new feature broke a lot of my plots too. limits = force fixed it, but I don't actually understand what it does.

I think the old behaviour was better, where one could define a list of colors to be used in all kinds of plots, without needing to add additional paramters to the scale_ functions.

@teunbrand
Copy link
Collaborator

To expand a bit on the workaround; scales can take functions or values as the limits. The function takes the computed (standard) limits as input. Using limits = force uses the force function to return the computed limits unchanged. Because the limits are now a function (and not NULL), the following branch (from #4511 (comment)) isn't entered:

ggplot2/R/scale-manual.r

Lines 135 to 137 in de5d63f

if (is.null(limits)) {
limits <- names(values)
}

Which skips grabbing the limits from the named manual values.

In addition: perhaps, if the aim of #4471 was to allow fewer elements, then the line above could be changed to:

if (is.null(limits)) { 
  force(values)
  limits <- function(x) {intersect(x, names(values))}
} 

Which appears to be an easier solution than the one proposed in #4547, wherein the Scale ggproto object is altered for every scale.

@yutannihilation
Copy link
Member

Fixed by #4619

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug an unexpected problem or unintended behavior
Projects
None yet
Development

No branches or pull requests

7 participants