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

Fill and color guides for factor with drop=FALSE don't show color. #5869

Closed
ltierney opened this issue Apr 26, 2024 · 8 comments · Fixed by #6037
Closed

Fill and color guides for factor with drop=FALSE don't show color. #5869

ltierney opened this issue Apr 26, 2024 · 8 comments · Fixed by #6037
Labels
documentation tidy-dev-day 🤓 Tidyverse Developer Day

Comments

@ltierney
Copy link

In ggplot2 3.5.1 the fill guide for a factor where not all level sare present but drop=FALSE is used shows the label for the missing level but not the color. A simple example:

library(ggplot2)
d <-data.frame(x = factor(1:3, levels = 1:4), y = 1:3)
ggplot(d, aes(x, y, fill = x)) + 
    geom_col() +
    scale_fill_discrete(drop = FALSE)

This was different in the version available a year ago. Compare
this from 2024 to this from 2023.

@teunbrand
Copy link
Collaborator

Thanks for the report! This appears to be a duplicate of #5728. The summary of that issue is: we have no intention of changing this behaviour.

@teunbrand teunbrand closed this as not planned Won't fix, can't repro, duplicate, stale Apr 26, 2024
@ltierney
Copy link
Author

It's a pretty clear regression, something I try to avoid in software I write and maintain. But it's your software and your call.

@teunbrand
Copy link
Collaborator

I don't think of it as a regression but as trade-off that trades the complex problem of having to tailor guide_legend(override.aes) for the mild inconvenience of having to put show.legend = TRUE.

@ltierney
Copy link
Author

When existing working code no longer works correctly after a change that is, by definition, a regression. Something we try pretty hard to avoid in maintaining R — not that we always succeed.

Thanks for the show.legend = TRUE hint; putting that in the right places works for me. I'm glad you told me since there is no way I can see I could have figured out from the documentation of show.legend in ?geom_polygon or ?geom_sf that that would give me what I needed.

@teunbrand
Copy link
Collaborator

teunbrand commented Apr 26, 2024

When existing working code no longer works correctly after a change

Argueably, the code was working incorrectly before by showing legend keys that weren't present in the data. I get that this might be intended, but this intent has to be explicit now.

there is no way I can see I could have figured out from the documentation

As mentioned in #5728, it was displayed in the changelog, described in the release blog and we've since included it in the documentation of the drop argument.

@ltierney
Copy link
Author

When existing working code no longer works correctly after a change

Argueably, the code was working incorrectly before by showing legend keys that weren't present in the data. I get that this might be intended, but this intent has to be explicit now.''

We sometimes try to make that argument as well; doesn't usually work very well for us either :-).

there is no way I can see I could have figured out from the documentation

As mentioned in #5728, it was displayed in the changelog, described in the release blog and we've since included it in the documentation of the drop argument.

That looks good. might be worth having a cross-reference from the show.legend docs as well.

The current behavior with just drop = FALSE still seems very odd to me. The guide does include all labels, just not all colors. It may make sense with the internal design, but still seems odd.

In any case I think we've both spent enough time on this.

@teunbrand
Copy link
Collaborator

That looks good. might be worth having a cross-reference from the show.legend docs as well.

Thanks for this suggestion, I'll reopen to remind us to document this.

@teunbrand teunbrand reopened this Apr 26, 2024
@teunbrand teunbrand added documentation tidy-dev-day 🤓 Tidyverse Developer Day labels Apr 26, 2024
@ssinari
Copy link

ssinari commented Jun 3, 2024

In general, legends are used to show the scale of the measurements and not just the realized values. Additionally, completing the dataframe with respect to the variable being plotted provides the full, correctly formatted legend, without the explicit show.legend = TRUE. So in the latter case, intent can still be provided implicitly, nullifying the expectation that lead to this change.

I support the thinking that this behavior is indeed odd and not very useful. Hope you rethink on such an important issue as it breaks most previous scripts and behavior.
Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation tidy-dev-day 🤓 Tidyverse Developer Day
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants