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

Get and check aes params #23

Closed
skaltman opened this issue Nov 17, 2021 · 6 comments · Fixed by #28
Closed

Get and check aes params #23

skaltman opened this issue Nov 17, 2021 · 6 comments · Fixed by #28
Assignees
Labels
enhancement New feature or request

Comments

@skaltman
Copy link
Contributor

It would be useful to be able to check for aesthetic parameters (i.e., aesthetics that have been set and not mapped), such as fill = "blue":

library(ggcheck)
library(ggplot2)

p <-
  ggplot(mpg, aes(manufacturer)) +
  geom_bar(fill = "blue", width = 0.5)

Created on 2021-11-16 by the reprex package (v2.0.1)

uses_geom_param() checks "geom params", (binwidth, etc.), but not "aes params" (fill = "blue"):

uses_geom_param(p, "bar", params = list(fill = "blue"))
#> Error in uses_geom_param(p, "bar", params = list(fill = "blue")): Grading error: the supplied parameters 'fill' are invalid.
uses_geom_param(p, "bar", params = list(width = 0.5))
#> [1] TRUE

It would also be nice to be able to easily get aes params. You can do:

get_geom_layer(p, geom = "bar")$layer$aes_params
#> $fill
#> [1] "blue"

but it would be nice to have a function.

@rossellhayes rossellhayes self-assigned this Nov 17, 2021
@rossellhayes rossellhayes added the enhancement New feature or request label Nov 17, 2021
@rossellhayes
Copy link
Contributor

Would you envision this as one function that can check both geom params and aes params, like uses_geom_param(p, "bar", params = list(fill = "blue")), or a new function to check aes params, like uses_aes_param()?

@skaltman
Copy link
Contributor Author

I think it would be easier if it was one function, but it does mess with the naming convention a bit since fill is not technically a geom param.

It would also be nice if the function used ... so you don't have to put the parameters in a list (like what you did with the labels functions).

@rossellhayes
Copy link
Contributor

Looking at the documentation, it seems that the function is already a bit overloaded. It currently checks for geom_params and stat_params, so I don't think adding aes_params would cause much issue.

@rossellhayes
Copy link
Contributor

@skaltman Currently, uses_labels() returns a named vector if you check more than one label. uses_geom_params() returns a single logical value indicating if all params match. Should uses_geom_params() return a named vector as well?

library(ggcheck)
library(ggplot2)

p <- ggplot(mpg, aes(x = manufacturer, y = hwy)) +
  geom_boxplot(width = 0.5, na.rm = TRUE) +
  labs(x = "Manufacturer", y = "MPG")

uses_labels(p, x = "Manufacturer", y = "MPG")
#>    x    y 
#> TRUE TRUE
uses_geom_param(p, "boxplot", width = 0.5, na.rm = TRUE)
#> [1] TRUE

Created on 2021-11-30 by the reprex package (v2.0.1)

@skaltman
Copy link
Contributor Author

@rossellhayes Yeah, I think a named vector would work well. It seems like it would be good if both functions worked similarly. And if uses_geom_param() returns a named vector, you can check where exactly the student code went wrong without writing multiple calls to uses_geom_param().

@rossellhayes
Copy link
Contributor

@skaltman Thank you! The PR should now be ready to go, let me know how it looks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants