ggplot2 icon indicating copy to clipboard operation
ggplot2 copied to clipboard

fix: obtain expected quantity values from manual scale

Open liang09255 opened this issue 2 years ago • 2 comments

This PR fixes an error in the code and adds corresponding unit tests.

When using manual_scale to create a discrete_scale, the palette should be a function that can return the specified number of values when called with a single integer argument.

Here is the parameter description excerpt from scale-.R:

@param palette A palette function that when called with a single integer argument (the number of levels in the scale) returns the values that they should take (e.g., [scales::hue_pal()]).

In the original manual_scale method, it simply returned all the values. However, when values are not named, we only need to return the corresponding first 'n' values. Therefore, the modified code includes a condition to handle this case.

liang09255 avatar Jul 25 '23 06:07 liang09255

The case where a palette returns more values than requested by n is handled internally in ScaleDiscrete$map(). Aside from the manual palette, this also handles other cases for e.g. custom palettes:

library(ggplot2)

ggplot(mpg, aes(displ, hwy, colour = factor(cyl))) +
  geom_point() +
  discrete_scale(
    "colour", "colour",
    palette = function(n) rainbow(10) # more than the 4 needed for `factor(cyl)`
  )

Created on 2023-07-26 with reprex v2.0.2

What I'm not understanding here is why this should be fixed if this case is dealt with already.

teunbrand avatar Jul 26 '23 18:07 teunbrand

Thank you for your patient explanation, i realize that the reason why i think it is a bug is that i incorrectly use "scale_color_manual"

But I still believe that this PR can improve the robustness of the code without affecting other functions

library(ggplot2)
library(ggsci)


plot_palette <- function(name){
  if (name %in% c("npg", "jco", "igv")){
    eval(parse(text = sprintf("ggsci::scale_colour_%s()", name)))
  }else if (name == "traffic_light"){
    # The reason why i make this mistake is i wrongly think 
    # "scale_color_manual" is the wrapped for "discrete_scale" 
    # which can create a color palette conveniently
    
    # This is my incorrect code causing a bug
    scale_color_manual(values = c("red", "green", "yellow", "blue", "black"))
    
    # This is the correct code
    # discrete_scale("color", "color", function(n){
    #   colors <- c("red", "green", "yellow", "blue", "black")
    #   colors[1:n]
    # })
  }else{
    stop("wrong name") 
  }
}

palette_name <- "traffic_light"
# palette_name <- "jco"

palette <- plot_palette(palette_name)

# when i using my function, it can correctly draw the image
ggplot(mpg, aes(displ, hwy, colour = factor(cyl))) +
  geom_point() +
  palette

# When I try to output the colors used in the drawing, it output all of the colors
cat(palette$palette(length(unique(mpg$cyl))))

# traffice_light output all the color: 
#   red green yellow blue black

# jco output first 4 color: 
#   #0073C2FF #EFC000FF #868686FF #CD534CFF

liang09255 avatar Jul 27 '23 08:07 liang09255