From ce46f2c50b44456bda92f09fdaa7599d842be7fa Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Sat, 6 Jun 2020 19:31:36 +0800 Subject: [PATCH 1/2] when as.data.frame dispatches to list method, force input to list --- NEWS.md | 2 ++ R/as.data.table.R | 3 ++- inst/tests/tests.Rraw | 14 ++++++++++++-- 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/NEWS.md b/NEWS.md index efc1a43ca6..3edd9dad17 100644 --- a/NEWS.md +++ b/NEWS.md @@ -115,6 +115,8 @@ unit = "s") 16. Non-equi joins now automatically set `allow.cartesian=TRUE`, [4489](https://github.com/Rdatatable/data.table/issues/4489). Thanks to @Henrik-P for reporting. +17. `as.data.table` more robust to unrecognized subclasses of `data.frame`, [#4526](https://github.com/Rdatatable/data.table/issues/4526). Thanks @RicoDiel for the report. + ## NOTES 0. Retrospective license change permission was sought from and granted by 4 contributors who were missed in [PR#2456](https://github.com/Rdatatable/data.table/pull/2456), [#4140](https://github.com/Rdatatable/data.table/pull/4140). We had used [GitHub's contributor page](https://github.com/Rdatatable/data.table/graphs/contributors) which omits 3 of these due to invalid email addresses, unlike GitLab's contributor page which includes the ids. The 4th omission was a PR to a script which should not have been excluded; a script is code too. We are sorry these contributors were not properly credited before. They have now been added to the contributors list as displayed on CRAN. All the contributors of code to data.table hold its copyright jointly; your contributions belong to you. You contributed to data.table when it had a particular license at that time, and you contributed on that basis. This is why in the last license change, all contributors of code were consulted and each had a veto. diff --git a/R/as.data.table.R b/R/as.data.table.R index 308a7b2ffe..3b75885e6a 100644 --- a/R/as.data.table.R +++ b/R/as.data.table.R @@ -219,7 +219,8 @@ as.data.table.data.frame = function(x, keep.rownames=FALSE, key=NULL, ...) { } if (any(vapply_1i(x, function(xi) length(dim(xi))))) { # not is.atomic because is.atomic(matrix) is true # a data.frame with a column that is data.frame needs to be expanded; test 2013.4 - return(as.data.table.list(x, keep.rownames=keep.rownames, ...)) + # x may class with [[ method that behaves differently, so as.list to for default [[, #4526 + return(as.data.table.list(as.list(x), keep.rownames=keep.rownames, ...)) } ans = copy(x) # TO DO: change this deep copy to be shallow. setattr(ans, "row.names", .set_row_names(nrow(x))) diff --git a/inst/tests/tests.Rraw b/inst/tests/tests.Rraw index e28885fb4e..83cbebf33e 100644 --- a/inst/tests/tests.Rraw +++ b/inst/tests/tests.Rraw @@ -16945,5 +16945,15 @@ if (.Platform$OS.type=="windows") local({ test(2143, rbind(DT,list(c=4L,a=7L)), error="2.*1.*c.*1") }) # test back to English (the argument order is back to 1,c,2,1) -test(2144, rbind(DT,list(c=4L,a=7L)), error="Column 1 ['c'] of item 2 is missing in item 1") - +test(2144, rbind(DT,list(c=4L,a=7L)), error="Column 1 ['c'] of item 2 is missing in item 1") + +# as.data.table.list edits list elements, so must be sure +# x does not use some other `[[` method, #4526 +x = data.frame(a = 1:5) +x$b = matrix(6:15, ncol=2L) +class(x) = c('foo', 'data.frame') +`[[.foo` = function(x, i) { + if (any(sapply(x, inherits, 'data.table'))) stop('failure') + as.list(x)[[i]] +} +test(2145, as.data.table(x), data.table(a=1:5, b.V1=6:10, b.V2=11:15)) From 84efb7e6f90cc277c263e4fa2de80d33755dcc4a Mon Sep 17 00:00:00 2001 From: Matt Dowle Date: Tue, 27 Apr 2021 13:04:49 -0600 Subject: [PATCH 2/2] tweak news item; see discussion in PR --- NEWS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index 876979cb10..edfb3b8a4d 100644 --- a/NEWS.md +++ b/NEWS.md @@ -57,7 +57,7 @@ 6. When `j` returns an object whose class `"X"` inherits from `data.table`; i.e. class `c("X", "data.table", "data.frame")`, the derived class `"X"` is no longer incorrectly dropped from the class of the `data.table` returned, [#4324](https://github.com/Rdatatable/data.table/issues/4324). Thanks to @HJAllen for reporting and @shrektan for the PR. -7. `as.data.table()` is more robust to subclasses of `data.frame` which may have different `[[` methods, leading to errors, [#4526](https://github.com/Rdatatable/data.table/issues/4526). Thanks @RicoDiel for the report. +7. `as.data.table()` failed with `.subset2(x, i, exact = exact): attempt to select less than one element in get1index` when passed an object inheriting from `data.table` with a different `[[` method, such as the class `dfidx` from the `dfidx` package, [#4526](https://github.com/Rdatatable/data.table/issues/4526). Thanks @RicoDiel for the report, and Michael Chirico for the PR. ## NOTES