Skip to content

Manual scale limits default to present names of values #4619

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.

Sign up for GitHub

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 11 commits into from
Sep 20, 2021
Merged

Manual scale limits default to present names of values #4619

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
59850ac
Merge pull request #1 from tidyverse/master
teunbrand Jan 13, 2021
86787de
Merge branch 'tidyverse:master' into master
teunbrand Sep 15, 2021
fe5953e
manual scale limits default to present names of values
teunbrand Sep 15, 2021
fa042fc
Don't force values (again)
teunbrand Sep 16, 2021
ba53004
Add news bullet
teunbrand Sep 18, 2021
c0877d7
Trim mentioned issues
teunbrand Sep 19, 2021
4635800
Omit brackets
teunbrand Sep 19, 2021
3671329
Add small comment
teunbrand Sep 19, 2021
36dd200
Merge branch 'tidyverse:master' into manual_scale_limits
teunbrand Sep 19, 2021
3027023
Remove trailing comma from test
teunbrand Sep 19, 2021
91bad65
Rephrase news bullet
teunbrand Sep 20, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions NEWS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# ggplot2 (development version)

* `scale_*_manual()` no longer displays extra legend keys, or changes their
order, when a named `values` argument has more items than the data. The
previous behaviour can be replicated by using
`scale_*_manual(values = vals, limits = names(vals))`. (@teunbrand, @banfai,
#4511, #4534)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The word "previous behaviour" might be a bit confusing to those who want further previous behaviour, i.e. behaviour before v3.3.4. How about describing it a bit more explicitly something like "To display all the values on the legend, use scale_*_manual(values = vals, limits = names(vals))"? What do you think?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, "previous behavior" is confusing. Better to describe the specific behavior. I like the proposed sentence.


# ggplot2 3.3.5
This is a very small release focusing on fixing a couple of untenable issues
that surfaced with the 3.3.4 release
Expand Down
5 changes: 3 additions & 2 deletions R/scale-manual.r
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,9 @@ manual_scale <- function(aesthetic, values = NULL, breaks = waiver(), ..., limit
force(values)
}

if (is.null(limits)) {
limits <- names(values)
if (is.null(limits) && !is.null(names(values))) {
# Limits as function to access `values` names later on (#4619)
limits <- function(x) intersect(x, names(values))
}

# order values according to breaks
Expand Down
2 changes: 1 addition & 1 deletion tests/testthat/test-scale-manual.r
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ test_that("fewer values (#3451)", {
test_that("limits and breaks (#4619)", {
# values don't change legend order
s1 <- scale_colour_manual(
values = c("8" = "c", "4" = "a", "6" = "b"),
values = c("8" = "c", "4" = "a", "6" = "b")
)
s1$train(c("8", "6", "4"))
expect_equal(s1$map(c("8", "6", "4")), c("c", "b", "a"))
Expand Down