Skip to content

allow fewer elements in named values vector in manual scales #4471

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

Merged
merged 9 commits into from
May 10, 2021

Conversation

thomasp85
Copy link
Member

Fix #3451

This PR relaxes the requirement on the manual scale values vector and allow it to contain fewer elements than the data. Non-existing values are implicitly considered NA and mapped to the na.value value

@thomasp85 thomasp85 added this to the ggplot2 3.3.4 milestone May 7, 2021
@paleolimbot
Copy link
Member

This is close but I'm not sure it's exactly what you intended...I imagine that specifying values as a named vector should either match the equivalent breaks + values or limits + values, and after this PR that doesn't seem to be the case (at least with the guide). I seem to remember there was an upstream scales thing here too so maybe it's outside the control of this code. Reprexes:

CRAN ggplot2:

library(ggplot2)
#> Warning: package 'ggplot2' was built under R version 4.0.5

df <- data.frame(x = 1, y = 1:3, z = factor(c("a","b", NA), exclude = NULL))
p <- qplot(x, y, data = df, colour = z)

p + scale_color_manual(limits = "a", values = "black")
#> Warning: Removed 2 rows containing missing values (geom_point).

p + scale_colour_manual(breaks = "a", values = "black")
#> Error: Insufficient values in manual scale. 2 needed but only 1 provided.
p + scale_colour_manual(values = c("a" = "black"))
#> Error: Insufficient values in manual scale. 2 needed but only 1 provided.

Created on 2021-05-07 by the reprex package (v2.0.0)

This PR:

library(ggplot2)

df <- data.frame(x = 1, y = 1:3, z = factor(c("a","b", NA), exclude = NULL))
p <- qplot(x, y, data = df, colour = z)

p + scale_color_manual(limits = "a", values = "black")
#> Warning: Removed 2 rows containing missing values (geom_point).

p + scale_colour_manual(breaks = "a", values = "black")
#> Warning: Removed 2 rows containing missing values (geom_point).

p + scale_colour_manual(values = c("a" = "black"))
#> Warning: Removed 2 rows containing missing values (geom_point).

Created on 2021-05-07 by the reprex package (v2.0.0)

@thomasp85
Copy link
Member Author

What is the behaviour you'd want? This change means that unmatched values gets the na value (defaults to NA which means they won't get drawn)

@paleolimbot
Copy link
Member

It looks like the behaviour is the same but the legend is drawn differently for the named values. It looks like the behaviour and legend for limits + values and breaks + values is the same for this example, but I'd expect it to match limits + values since breaks is theoretically only for the guide (I think).

@thomasp85 thomasp85 requested a review from paleolimbot May 7, 2021 12:41
@paleolimbot
Copy link
Member

The output is now consistent, but I think this no longer fixes the issue (all three error now).

library(ggplot2)

df <- data.frame(x = 1, y = 1:3, z = factor(c("a","b", NA), exclude = NULL))
p <- qplot(x, y, data = df, colour = z)

p + scale_color_manual(limits = "a", values = "black")
#> Error: Insufficient values in manual scale. 2 needed but only 1 provided.
p + scale_colour_manual(breaks = "a", values = "black")
#> Error: Insufficient values in manual scale. 2 needed but only 1 provided.
p + scale_colour_manual(values = c("a" = "black"))
#> Error: Insufficient values in manual scale. 2 needed but only 1 provided.

Created on 2021-05-07 by the reprex package (v2.0.0)

@thomasp85
Copy link
Member Author

Why am I so bad at this today...

@thomasp85
Copy link
Member Author

Ah - forgot to push the fix, sorry

@clauswilke
Copy link
Member

I've looked at the code but I have no idea why the patch does what it does. If @paleolimbot thinks it's good I'm fine with that.

@paleolimbot
Copy link
Member

All good!

I'm not sure how deeply you want to delve into the issue...this PR is a huge improvement over the current funky manual scale, but the manual scale is still inconsistent (I would argue) with the behaviour of other discrete scales. Is it easy to have the NA be drawn (i.e., mapped to na.value) instead of removed?

library(ggplot2)

df <- data.frame(x = 1, y = 1:3, z = factor(c("a","b", NA), exclude = NULL))
p <- qplot(x, y, data = df, colour = z)

p + scale_colour_discrete(limits = "a")

p + scale_colour_discrete(breaks = "a")

p + scale_color_manual(limits = "a", values = "black")
#> Warning: Removed 2 rows containing missing values (geom_point).

p + scale_colour_manual(breaks = "a", values = "black")
#> Error: Insufficient values in manual scale. 2 needed but only 1 provided.
p + scale_colour_manual(values = c("a" = "black"))
#> Warning: Removed 2 rows containing missing values (geom_point).

Created on 2021-05-07 by the reprex package (v2.0.0)

@thomasp85
Copy link
Member Author

You just set na.value to something sensible, e.g. "grey"

I'm open to setting a better default that matches the default discrete scales though

@paleolimbot
Copy link
Member

I see, you need p + scale_colour_manual(values = c("a" = "black"), na.value = "grey") to make it work. I think it'd be more consistent if na.value was set to the same thing as the default scale but no strong feelings!

@thomasp85 thomasp85 merged commit c9adeed into master May 10, 2021
banfai added a commit to banfai/ggplot2 that referenced this pull request 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 pull request Jul 7, 2021
* issue introduced in tidyverse#4471
* potential solution for tidyverse#4534 and tidyverse#4511
* add extra tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

scale_manual throws unexpected error when na.values given and insufficient values provided
3 participants