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

Check ellipses in several places #6031

Merged
merged 7 commits into from
Aug 28, 2024
Merged

Conversation

teunbrand
Copy link
Collaborator

@teunbrand teunbrand commented Aug 6, 2024

This PR aims to fix #3196.

Briefly, there are several places where funneling arguments through ... is now checked.
These function now perform checks:

  • fortify()
  • get_alt_text()
  • labs()

#3196 (comment) also identified guides() and labeller() as function that should but do not perform ... checks, but these are somewhat more complicated.

Reprex from the comment:

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

# fortify now warns
p <- ggplot(mpg, aes(displ, hwy, colour = drv), foo = "bar") +
  geom_point() +
  guides(
    # legends now warn
    colour = guide_legend(non = "sense"),
    # non existing aesthetics are still silently accepted
    foobar = "qux" 
  ) +
  facet_wrap(
    ~ year,
    # labeller args are still silently accepted
    labeller = labeller(year = label_both, nonsense = label_value)
  ) + 
  labs(
    alt = "my alt text",
    # unknown labels are no longer ignored
    i_dont = "exist"
  )
#> Warning in fortify(data, ...): Arguments in `...` must be used.
#> ✖ Problematic argument:
#> • foo = "bar"
#> ℹ Did you misspell an argument name?
#> Warning in guide_legend(non = "sense"): Arguments in `...` must be used.
#> ✖ Problematic argument:
#> • non = "sense"
#> ℹ Did you misspell an argument name?

# Labels are checked during build time
p <- ggplot_build(p)
#> Warning: Ignoring unknown labels:
#> • `i_dont = "exist"`

# Extra arguments are no longer ignored
get_alt_text(p, hamspam = "green eggs")
#> Warning in get_alt_text(p, hamspam = "green eggs"): Arguments in `...` must be used.
#> ✖ Problematic argument:
#> • hamspam = "green eggs"
#> ℹ Did you misspell an argument name?
#> [1] "my alt text"

Created on 2024-08-06 with reprex v2.1.1

Copy link
Member

@thomasp85 thomasp85 left a comment

Choose a reason for hiding this comment

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

LGTM

@teunbrand teunbrand merged commit a49bf1e into tidyverse:main Aug 28, 2024
13 checks passed
@teunbrand teunbrand deleted the check_ellipses branch August 28, 2024 07:24
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.

Checking ...
2 participants