-
Notifications
You must be signed in to change notification settings - Fork 2.1k
cur_data() / cur_data_all() not simplifying list columns
#6020
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
Conversation
…ata frames. closes #5901
cur_data() / cur_data_all() not simplifying list columns
|
However, the data is still simplified as usual when used directly in the expression, or through library(dplyr, warn.conflicts = FALSE)
library(tibble)
df <- tibble(x = list(tibble(a = 1), tibble(a = 2))) %>%
rowwise()
summarise(df, is_tibble(x))
#> # A tibble: 2 x 1
#> `is_tibble(x)`
#> <lgl>
#> 1 TRUE
#> 2 TRUE
summarise(df, across(x, is_tibble))
#> # A tibble: 2 x 1
#> x
#> <lgl>
#> 1 TRUE
#> 2 TRUECreated on 2021-09-17 by the reprex package (v2.0.0) |
|
|
||
| pick = function(vars) { | ||
| cols <- self$current_cols(vars) | ||
| if (inherits(private$data, "rowwise_df")) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you explain why we have to re-wrap in a list? (As opposed to not unwrapping somewhere?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It happens in this line:
Line 20 in bb84360
| if (rowwise && vctrs::vec_is_list(column) && Rf_length(column) > 0) { |
void dplyr_lazy_vec_chop_grouped(SEXP chops_env, SEXP rows, SEXP data, bool rowwise) {
SEXP names = PROTECT(Rf_getAttrib(data, R_NamesSymbol));
R_xlen_t n = XLENGTH(data);
const SEXP* p_data = VECTOR_PTR_RO(data);
const SEXP* p_names = STRING_PTR_RO(names);
for (R_xlen_t i = 0; i < n; i++) {
SEXP prom = PROTECT(Rf_allocSExp(PROMSXP));
SET_PRENV(prom, R_EmptyEnv);
SEXP column = p_data[i];
if (rowwise && vctrs::vec_is_list(column) && Rf_length(column) > 0) {
SET_PRCODE(prom, column);
} else {
SET_PRCODE(prom, Rf_lang3(dplyr::functions::vec_chop, column, rows));
}
SET_PRVALUE(prom, R_UnboundValue);
Rf_defineVar(rlang::str_as_symbol(p_names[i]), prom, chops_env);
UNPROTECT(1);
}
UNPROTECT(1);
}When we setup the promise that makes the chops, and this is a rowwise data, we can skip using vec_chop() and simply use the list columns as the chops.
This gives us the rowwise simplification for when we refer to the list column in an expression (not having to [[1]]).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
... and for non-original columns, i.e. those that are added, they go through the <DataMask>$add_one() method:
add_one = function(name, chunks, result) {
if (inherits(private$data, "rowwise_df")){
is_scalar_list <- function(.x) {
vec_is_list(.x) && length(.x) == 1L
}
if (all(map_lgl(chunks, is_scalar_list))) {
chunks <- map(chunks, `[[`, 1L)
}
}
.Call(`dplyr_mask_add`, private, name, result, chunks)
},There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah makes sense. Thanks!
Co-authored-by: Hadley Wickham <[email protected]>
closes #5901
Created on 2021-09-17 by the reprex package (v2.0.0)