search

tidyverse / tibble

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

feat: `as_tibble()` calls `as.data.frame()` for objects that are not subclasses of `"tbl_df"` #1557

Closed TimTaylor closed 10 months ago

TimTaylor commented 11 months ago

This patch ensures as_tibble.data.frame first calls as.data.frame to allow for methods provided by extended data frame classes. The idea was proposed by Jan Gorecki in https://github.com/Rdatatable/data.table/issues/5698.

Fixes #1556.

TimTaylor commented 11 months ago

Failing on pre 4.0.0 release due to use of .S3method() which is "new". Unsure if this is an issue for you or not as the usage is only within the test, not package, code.

krlmlr commented 11 months ago

I suspect it's worth understanding the dplyr failure -- the other failures could be a consequence of that.

TimTaylor commented 11 months ago

I suspect it's worth understanding the dplyr failure -- the other failures could be a consequence of that.

hmm - I hadn't seen that there was no explicit method for tbl_df.

Now fixed (hopefully) with latest commit. I've rechecked dplyr and seems ok so :crossed_fingers: . Note the actions are currently failing but not due to package code.

krlmlr commented 11 months ago

Perhaps we want a separate as_tibble.tbl_df() method that would do less work? I vaguely remember this existed at some point.

It's always amazing to see how an innocuous change can quickly turn into a deep rabbit hole. I can't spend too much time digging myself, but happy to assist.

I'm seeing the GHA failure elsewhere, need to fix it.

TimTaylor commented 11 months ago

Yeah - I wondered that too but didn't want to make the hole any bigger. I think one or two other packages (e.g. LexisNexisTools) may be genuine breakages (only due to an unneeded attribute that has been tested against). I'll dig in to these over the next few days and update with any info. No rush to get this sorted - we will get there in the end.

TimTaylor commented 11 months ago

@krlmlr - I've looked through the revdep problems in more detail and I'm pretty sure it is only {LexisNexisTools} that will need patching. Is it possible to bump the actions and then if possible your revdep checks? (tests failed after https://github.com/tidyverse/tibble/pull/1557/commits/23dd204abfe7b234e391b0089f341bca2e619127 due to the r-project domain issue).

krlmlr commented 10 months ago

I wonder what's wrong with the GHA checks. The main branch looks good.

TimTaylor commented 10 months ago

Happy to poke at the revdeps again if it's not too costly and they can be rerun? (unsure on your CI setup for the gazillion revdeps you have).

krlmlr commented 10 months ago

Thanks. I think it's good as is, I'll wait for a bit until the CRAN release.