tidyverse / tibble

A modern re-imagining of the data frame
https://tibble.tidyverse.org/
Other
671 stars 130 forks source link

Could `as_tibble.data.frame()` be stricter? #1556

Closed TimTaylor closed 10 months ago

TimTaylor commented 11 months ago

Currently as_tibble.data.frame() treats all extended data frames (class = <mydfclass, data.frame>) as if they were normal data frames (class = ) and in turn ignores any as.data.frame() method that the object has available. A consequence of this manifested in https://github.com/tidyverse/tibble/issues/1555, and the follow up report in https://github.com/Rdatatable/data.table/issues/5698, where additional attributes (only of use to {data.table}) were not removed on coercion.

In https://github.com/Rdatatable/data.table/issues/5698, Jan proposed the following small addition as a possible solution:

as_tibble.data.frame = function(x, ...) {
  if (!identical(class(x), "data.frame")) return(as_tibble(as.data.frame(x)))
  ... # continue old as_tibble.data.frame body
}

This benefits not just {data.table} but any package that provides an extended object class. The onus is still on those packages to provide an as.data.frame() method but hopefully this is something they are already doing.

Happy to send a PR if receptive?

krlmlr commented 11 months ago

Thanks. Off the top of my head, this doesn't look too bad. Should we instead:

as_tibble.data.frame <- function(x, ...) {
  if (!identical(class(x), "data.frame")) {
    x <- as.data.frame(x)
  }
  ... # continue old as_tibble.data.frame body
}

We can see the consequences when running revdepchecks.

TimTaylor commented 11 months ago

Oh yeah - that's nicer as we're already in the relevant method - cheers!