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

Vignette: discuss optional string input #6215

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

teunbrand
Copy link
Collaborator

This PR aims to fix #6208.

If somebody has better advice for dealing with optional character-input, I'd be glad to adjust.

@yjunechoe
Copy link
Contributor

Tangentially related, but I wonder whether rlang::data_sym() passing NULL through could resolve some of this headache:

rlang::data_sym(NULL)
#> NULL

Instead of its current behavior:

rlang::data_sym(NULL)
#> Error in `sym()`:
#> ! Can't convert `NULL` to a symbol.

Especially because I feel like this "optional character-input" problem has already been solved in tidyselect (with all_of()/any_of()), so it'd be nice if data_sym()/data_syms() could let us get something close to tidyselect semantics within tidyeval contexts (like aes()) by letting NULL pass through to mean an empty (NULL) selection. Ex:

df <- data.frame(x = 1)

# Tidyselect behavior
tidyselect_get <- function(df, col) {
  tidyselect::eval_select(tidyselect::all_of(col), df)
}
tidyselect_get(df, "x")
#> x 
#> 1
tidyselect_get(df, NULL)
#> named integer(0)

# Tidyeval with `data_sym()`
tidyeval_get <- function(df, col) {
  rlang::eval_tidy(rlang::data_sym(col), df)
}
tidyeval_get(df, "x")
#> [1] 1
tidyeval_get(df, NULL)
#> Error in `sym()`:
#> ! Can't convert `NULL` to a symbol.

# Tidyeval with modified `data_sym2()` that passes NULL through
data_sym2 <- \(x) if (!is.null(x)) rlang::data_sym(x)
tidyeval_get2 <- function(df, col) {
  rlang::eval_tidy(data_sym2(col), df)
}
tidyeval_get2(df, "x")
#> [1] 1
tidyeval_get2(df, NULL)
#> NULL

@teunbrand
Copy link
Collaborator Author

Thanks June, your comments make a lot of sense to me. I'd agree that this having data_sym() accept NULL would be the most elegant. I don't think ggplot2 should provide a user-exposed wrapper for this, as we'd have to maintain this into perpetuity. Rather, we could petition {rlang} if they'd be interested in this change. That allows us to include this PR as current advice and update the advice if and once we can use data_sym(NULL).

@yjunechoe
Copy link
Contributor

Rather, we could petition {rlang} if they'd be interested in this change.

Yeah, that's the intended build-up of my comment! Would be nice for the FR to be backed by a need in ggplot 😄

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.

Vignette ggplot2-in-packages does not cover programmatically passing NULL to aes()
2 participants