Skip to content
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

R-devel change to cbind/rbind re FAQ 2.24 #3948

Closed
mattdowle opened this issue Oct 8, 2019 · 2 comments · Fixed by #3968
Closed

R-devel change to cbind/rbind re FAQ 2.24 #3948

mattdowle opened this issue Oct 8, 2019 · 2 comments · Fixed by #3968
Milestone

Comments

@mattdowle
Copy link
Member

Received from Kurt and posted here with permission.
Fantastic we can finally remove this workaround. Haven't looked in detail yet.

====

Matt,

You may have seen that in R-devel we've changed the {c,r}bind S3
dispatch mechanisms with

r77192 | hornik | 2019-09-15 20:45:30 +0200 (Sun, 15 Sep 2019) | 1 line
Switch default for c77141.

r77141 | hornik | 2019-09-04 07:47:13 +0200 (Wed, 04 Sep 2019) | 1 line
Allow experimenting with {c,r}bind() S3 dispatch.

(with a cleanup for the new default coming up once I get to it).

With these changes, for S3 dispatch {c,r}bind now use the first S3
method found, so that binding data frames and data tables now works "as
intended" in the sense that you get the method of the object binding to.

E.g.,

require("data.table")

cbind.data.frame <-
function (..., deparse.level = 1)
data.frame(..., check.names = FALSE)

cbind.data.table <-
function (..., deparse.level = 1)
{
if (!identical(class(..1), "data.frame"))
for (x in list(...)) {
if (inherits(x, "data.table"))
return(data.table::data.table(...))
}
data.frame(..., check.names = FALSE)
}

DF <- data.frame(x=rep(c("b","a","c"),each=3), y=c(1,3,6), v=1:9)
DT <- data.table(x=rep(c("b","a","c"),each=3), y=c(1,3,6), v=1:9)

class(cbind(DF, DT))
class(cbind(DT, DF))

now gives

R> class(cbind(DF, DT))
[1] "data.frame"
R> class(cbind(DT, DF))
[1] "data.table" "data.frame"

Thus, for versions of R-devel later than r77192 you should now be able
to simply provide and register data.table methods for cbind and rbind
(instead of what you currently do in your startup code).

Can you please change data.table accordingly? Ideally, of course, as
quickly as possible ...

Best
-k

@mattdowle mattdowle added this to the 1.12.5 milestone Oct 8, 2019
@jangorecki
Copy link
Member

Great news, before merging such change we need to (manually) update windows R devel machine.

@mattdowle
Copy link
Member Author

mattdowle commented Oct 8, 2019

Perhaps the change can be made without a hard coded version number. The problem we've had before is that sometimes changes in R-devel don't get released as we expect. So our general goal is to try to detect the capability/change and branch on the detected behavior/capability instead of hard coding an assumption that a particular R version has that feature. Making the change in this way would mean master would work with older R-devel. In addition, we can turn back on AppVeyor R-devel which automatically uses latest daily R-devel.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants