NSE Arguments in functions calling plotly raise an error
Nobody has claimed this yet.
- Dominant language
- R
- Stars
- 2.7k
- Forks
- 641
- PR merge metrics
- No merged PRs in 30d
Description
While toying with different NSE approaches I stumbled upon the following behaviour which seems to me like a bug. Any NSE argument (even if not used in plotly) results in an error:
library(plotly)
nse_arg_raises_error <- function(xx) {
mf <- rlang::new_formula(NULL, rlang::ensym(xx), globalenv())
plot_ly(mtcars, x = ~ cyl, y = ~ mpg, type = "scatter",
mode = "markers")
}
nse_arg_raises_error() ## works
nse_arg_raises_error(cyl) ## does not work
#> Error in as.list.environment(x, all.names = TRUE): object 'cyl' not found
This behaviour is unique to plotly, b/c if I leave out the plot_ly call, the error does not happen.
it_is_a_plotly_problem <- function(xx) {
mf <- rlang::new_formula(NULL, rlang::ensym(xx), globalenv())
1 + 1
}
it_is_a_plotly_problem() ## works
#> [1] 2
it_is_a_plotly_problem(cyl) ## also works
#> [1] 2
However, some preliminary debugging shows that the culprit sits (deep) in base:::as.list.environment (which is called from lazyeval:::clone_env, which in turn together with its brethren from lazyeval is responsible for the NSE part in plot_ly)
it_is_a_as_list_environment_issue <- function(xx) {
mf <- rlang::new_formula(NULL, rlang::ensym(xx), globalenv())
cp <- as.list.environment(environment())
1 + 1
}
it_is_a_as_list_environment_issue(cyl)
#> Error in as.list.environment(x, all.names = TRUE): object 'cyl' not found
So basically, when lazyeval:::clone_env copies the function environment which contains an NSE argument this error from within the C code implementation of base::as.list.environment bubbles up.
I am inclined to say that this a feature and not a bug of as.list.environment and the user of it should make sure that environments using it are in fact able to be safely transformed into a list. Thus, I am posting the issue here as it touches the whole NSE strategy of plotly - coming from plotly:::eval_attr (especially since lazyeval is retired in favor of rlang).
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Reproduce the issue with the provided nse_arg_raises_error example, then trace plotly:::eval_attr into lazyeval:::clone_env and the call to base:::as.list.environment. Check how the plotly NSE path handles a function environment containing an unused NSE argument. Done means the demonstrated plot_ly call no longer raises the as.list.environment error; the payload names no test file.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- r
- Domain
- data-visualization
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100