jbkunst / highcharter

R wrapper for highcharts
http://jkunst.com/highcharter/
Other
719 stars 148 forks source link

Adjust mutate_mapping() to handle quosures #681

Closed ArtemSokolov closed 2 years ago

ArtemSokolov commented 3 years ago

Dear developers,

In a recent Stack Overflow question, it became apparent that hchart() is not handling quosures properly. I proposed a workaround involving ensym(), but @lionel- (one of the tidyeval leads) suggested that we submit a bug report, since ideally the highcharter functionality should work with quosures also.

I tracked the issue down to the following place in mutate_mapping(): https://github.com/jbkunst/highcharter/blob/d47a7792577588d0891dfd87a14a60914371e8c3/R/highcharts-api-add.R#L412-L416

where the list of expressions captured by haes() gets converted to character and then parsed back to expressions after getting annotated with names. This works well for expressions, but fails for quosures, which end up getting reinterpreted as formulas by this chain of operations.

As far as I can tell, the conversion to character is not really necessary, because mapping is already a named list. Therefore, lines 412-414 can be omitted, and line 416 can be replaced to use mapping directly:

data <- dplyr::mutate( data, !!!mapping )

A quick reprex using the data from the SO question referenced above:

## Data from the SO question
X <- structure(list(Year = c(2016, 2017, 2017, 2018, 2018, 2018),
    service = structure(c(10L, 3L, 9L, 5L, 7L, 9L), .Label = c("Defense Logistics Agency",
    "Chemical and Biological Defense Program", "Defense Information Systems Agency",
    "United States Special Operations Command", "Office of the Secretary Of Defense",
    "Missile Defense Agency", "Defense Advanced Research Projects Agency",
    "Navy", "Army", "Air Force"), class = "factor"), total = c(9.435,
    0, 10.442, 9.969, 73.759, 8.855)), row.names = c(NA, -6L), groups = structure(list(
    Year = c(2016, 2017, 2018), .rows = structure(list(1L, 2:3,
        4:6), ptype = integer(0), class = c("vctrs_list_of",
    "vctrs_vctr", "list"))), row.names = c(NA, 3L), class = c("tbl_df",
"tbl", "data.frame"), .drop = TRUE), class = c("grouped_df",
"tbl_df", "tbl", "data.frame"))

## Two mappings, one with expressions, another with quosures
m1 <- hcaes(x = !!rlang::expr(Year), y = !!rlang::expr(total), group = !!rlang::expr(service))
m2 <- hcaes(x = !!rlang::quo(Year), y = !!rlang::quo(total), group = !!rlang::quo(service))

## The current way works for m1, but not m2
tran <- as.character(m2)
newv <- names(m2)
list_names <- setNames(tran, newv) %>% lapply(rlang::parse_quo, env = globalenv())

dplyr::mutate(X, !!!list_names)
# Error: Problem with `mutate()` input `x`.
# ✖ Input `x` must be a vector, not a `formula` object.
# ℹ Input `x` is `~Year`.
# ℹ The error occured in group 1: Year = 2016.
# Run `rlang::last_error()` to see where the error occurred.

## Passing m1 or m2 directly to mutate solves the issue
dplyr::mutate(X, !!!m1)    # Works
dplyr::mutate(X, !!!m2)    # Also works
stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. Feel free to reopen it if you find it necessary.