RConsortium / S7

S7: a new OO system for R
https://rconsortium.github.io/S7
Other
386 stars 32 forks source link

Child class of `class_data.frame` doesn't play well with `{vctrs}` #367

Closed calderonsamuel closed 10 months ago

calderonsamuel commented 11 months ago

While I know is not mandatory to use vctrs based functions, I would expect S7 to work with them. Here I paste some comparison with an S3 class that works well with them. Base R functions seem to work normally. Is this a S7 issue or a vctrs issue?

library(S7)
library(dplyr)

foo_S3 <- function(x = data.frame()) {
    class(x) <- c("foo_S3", class(x))
    x
}

foo_S7 <- new_class("foo_S7", parent = class_data.frame)

base_df <- data.frame(x = seq(5, 1), y = letters[1:5], z = rep(c("A", "B"), times = c(2, 3)))
bar_S3 <- foo_S3(base_df) 
bar_S7 <- foo_S7(base_df)

select(bar_S3, x)
#>   x
#> 1 5
#> 2 4
#> 3 3
#> 4 2
#> 5 1
subset(bar_S7, select = x)
#>   x
#> 1 5
#> 2 4
#> 3 3
#> 4 2
#> 5 1
select(bar_S7, x)
#> Error in `eval_select_impl()`:
#> ! `x` must be a vector, not a <foo_S7/data.frame/S7_object> object.
#> Backtrace:
#>     ▆
#>  1. ├─dplyr::select(bar_S7, x)
#>  2. └─dplyr:::select.data.frame(bar_S7, x)
#>  3.   └─tidyselect::eval_select(expr(c(...)), data = .data, error_call = error_call)
#>  4.     └─tidyselect:::eval_select_impl(...)
#>  5.       └─vctrs::vec_assert(x)
#>  6.         └─vctrs:::stop_scalar_type(x, arg, call = call)
#>  7.           └─vctrs:::stop_vctrs(...)
#>  8.             └─rlang::abort(message, class = c(class, "vctrs_error"), ..., call = call)

mutate(bar_S3, baz = x + 1)
#>   x y z baz
#> 1 5 a A   6
#> 2 4 b A   5
#> 3 3 c B   4
#> 4 2 d B   3
#> 5 1 e B   2
transform(bar_S7, baz = x + 1)
#>   x y z baz
#> 1 5 a A   6
#> 2 4 b A   5
#> 3 3 c B   4
#> 4 2 d B   3
#> 5 1 e B   2
mutate(bar_S7, baz = x + 1)
#> Error in `vec_data()`:
#> ! `x` must be a vector, not a <foo_S7/data.frame/S7_object> object.
#> Backtrace:
#>      ▆
#>   1. ├─dplyr::mutate(bar_S7, baz = x + 1)
#>   2. ├─dplyr:::mutate.data.frame(bar_S7, baz = x + 1)
#>   3. │ ├─dplyr::dplyr_col_modify(.data, cols)
#>   4. │ └─dplyr:::dplyr_col_modify.data.frame(.data, cols)
#>   5. │   ├─base::as.list(vec_data(data))
#>   6. │   └─vctrs::vec_data(data)
#>   7. │     └─vctrs::obj_check_vector(x)
#>   8. └─vctrs:::stop_scalar_type(`<fn>`(`<foo_S7[,3]>`), "x", `<env>`)
#>   9.   └─vctrs:::stop_vctrs(...)
#>  10.     └─rlang::abort(message, class = c(class, "vctrs_error"), ..., call = call)

filter(bar_S3, x > 2)
#>   x y z
#> 1 5 a A
#> 2 4 b A
#> 3 3 c B
subset(bar_S7, x > 2)
#>   x y z
#> 1 5 a A
#> 2 4 b A
#> 3 3 c B
filter(bar_S7, x > 2) # same error with arrange()
#> Error in `vec_slice()`:
#> ! `x` must be a vector, not a <foo_S7/data.frame/S7_object> object.
#> Backtrace:
#>      ▆
#>   1. ├─dplyr::filter(bar_S7, x > 2)
#>   2. ├─dplyr:::filter.data.frame(bar_S7, x > 2)
#>   3. │ ├─dplyr::dplyr_row_slice(.data, loc, preserve = .preserve)
#>   4. │ └─dplyr:::dplyr_row_slice.data.frame(.data, loc, preserve = .preserve)
#>   5. │   ├─dplyr::dplyr_reconstruct(vec_slice(data, i), data)
#>   6. │   │ └─dplyr:::dplyr_new_data_frame(data)
#>   7. │   │   ├─row.names %||% .row_names_info(x, type = 0L)
#>   8. │   │   └─base::.row_names_info(x, type = 0L)
#>   9. │   └─vctrs::vec_slice(data, i)
#>  10. └─vctrs:::stop_scalar_type(`<fn>`(`<foo_S7[,3]>`), "x", `<env>`)
#>  11.   └─vctrs:::stop_vctrs(...)
#>  12.     └─rlang::abort(message, class = c(class, "vctrs_error"), ..., call = call)

# summmarise() and mutate() error with .by ----

aggregate(x ~ 1, data = bar_S7, sum)
#>    x
#> 1 15
summarise(bar_S7, baz = sum(x)) # this works
#>   baz
#> 1  15
summarise(bar_S7, baz = sum(x), .by = z) # this doesn't
#> Error in `eval_select_impl()`:
#> ! `x` must be a vector, not a <foo_S7/data.frame/S7_object> object.
#> Backtrace:
#>      ▆
#>   1. ├─dplyr::summarise(bar_S7, baz = sum(x), .by = z)
#>   2. └─dplyr:::summarise.data.frame(bar_S7, baz = sum(x), .by = z)
#>   3.   └─dplyr:::compute_by(...)
#>   4.     └─dplyr:::eval_select_by(by, data, error_call = error_call)
#>   5.       └─tidyselect::eval_select(...)
#>   6.         └─tidyselect:::eval_select_impl(...)
#>   7.           └─vctrs::vec_assert(x)
#>   8.             └─vctrs:::stop_scalar_type(x, arg, call = call)
#>   9.               └─vctrs:::stop_vctrs(...)
#>  10.                 └─rlang::abort(message, class = c(class, "vctrs_error"), ..., call = call)

# this works
bar_S7 |> 
    group_by(z) |> 
    mutate(baz = x + 1)
#> # A tibble: 5 × 4
#> # Groups:   z [2]
#>       x y     z       baz
#>   <int> <chr> <chr> <dbl>
#> 1     5 a     A         6
#> 2     4 b     A         5
#> 3     3 c     B         4
#> 4     2 d     B         3
#> 5     1 e     B         2

# this doesn't
bar_S7 |> 
    mutate(baz = x + 1, .by = z)
#> Error in `eval_select_impl()`:
#> ! `x` must be a vector, not a <foo_S7/data.frame/S7_object> object.
#> Backtrace:
#>      ▆
#>   1. ├─dplyr::mutate(bar_S7, baz = x + 1, .by = z)
#>   2. └─dplyr:::mutate.data.frame(bar_S7, baz = x + 1, .by = z)
#>   3.   └─dplyr:::compute_by(...)
#>   4.     └─dplyr:::eval_select_by(by, data, error_call = error_call)
#>   5.       └─tidyselect::eval_select(...)
#>   6.         └─tidyselect:::eval_select_impl(...)
#>   7.           └─vctrs::vec_assert(x)
#>   8.             └─vctrs:::stop_scalar_type(x, arg, call = call)
#>   9.               └─vctrs:::stop_vctrs(...)
#>  10.                 └─rlang::abort(message, class = c(class, "vctrs_error"), ..., call = call)

Created on 2023-09-21 with reprex v2.0.2

hadley commented 11 months ago

I think this is a vctrs issue; could you please file it there?