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

Violin resolution #5402

Merged
merged 3 commits into from
Sep 12, 2023
Merged

Violin resolution #5402

merged 3 commits into from
Sep 12, 2023

Conversation

teunbrand
Copy link
Collaborator

This PR aims to fix #5396.

Briefly, geom_violin() calls resolution() to determine the default width of a violin.
In resolution(), we're (correctly) making an exception for integers and 'mapped_discrete' vectors to have resolution 1.

However, the 'mapped_discrete' class got dropped from data$x due to a mean() operation.
In this PR, we skip the mean calculation when data$x only has 1 unique value, thereby preserving the 'mapped_discrete' class that gets forwarded to resolution().

Small demo:

devtools::load_all("~/packages/ggplot2/")
#> ℹ Loading ggplot2

dplyr::filter(diamonds, cut %in% c("Ideal", "Good")) |>
  ggplot(aes(price, cut)) +
  geom_violin() +
  scale_y_discrete(drop = FALSE)

Created on 2023-08-29 with reprex v2.0.2

Copy link
Member

@yutannihilation yutannihilation left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@teunbrand
Copy link
Collaborator Author

Thanks for the review!

@teunbrand teunbrand merged commit 0d2023d into tidyverse:main Sep 12, 2023
11 of 12 checks passed
@teunbrand teunbrand deleted the violin_resolution branch September 12, 2023 06:55
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.

Bug sometimes with polygon geoms, filtered factors, drop = FALSE, and width = NULL
2 participants