Skip to content

Commit

Permalink
prevent setDT on data.frame with matrix columns (#3770)
Browse files Browse the repository at this point in the history
  • Loading branch information
Michael Chirico authored and mattdowle committed Aug 28, 2019
1 parent d75265f commit 6a7246e
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 2 deletions.
2 changes: 2 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,8 @@
32. `x[, round(.SD, 1)]` and similar operations on the whole of `.SD` could return a locked result, incorrectly preventing `:=` on the result, [#2245](https://github.com/Rdatatable/data.table/issues/2245). Thanks @grayskripko for raising.
33. `setDT` now detects and halts if passed a `data.frame` containing matrix columns, [#3760](https://github.com/Rdatatable/data.table/issues/3760). Such objects cannot be converted to `data.table` by reference. The message suggests using `as.data.table` instead which will convert each matrix column to a new `data.table` column.
#### NOTES
1. `rbindlist`'s `use.names="check"` now emits its message for automatic column names (`"V[0-9]+"`) too, [#3484](https://github.com/Rdatatable/data.table/pull/3484). See news item 5 of v1.12.2 below.
Expand Down
7 changes: 6 additions & 1 deletion R/data.table.R
Original file line number Diff line number Diff line change
Expand Up @@ -2653,9 +2653,14 @@ setDT = function(x, keep.rownames=FALSE, key=NULL, check.names=FALSE) {
cname = as.character(name)
envir = home(cname, parent.frame())
if (bindingIsLocked(cname, envir)) {
stop("Can not convert '", cname, "' to data.table by reference because binding is locked. It is very likely that '", cname, "' resides within a package (or an environment) that is locked to prevent modifying its variable bindings. Try copying the object to your current environment, ex: var <- copy(var) and then using setDT again.")
stop("Cannot convert '", cname, "' to data.table by reference because binding is locked. It is very likely that '", cname, "' resides within a package (or an environment) that is locked to prevent modifying its variable bindings. Try copying the object to your current environment, ex: var <- copy(var) and then using setDT again.")
}
}
# check no matrix-like columns, #3760
colndim = vapply_1i(x, function(xi) length(dim(xi)))
if (any(idx <- colndim > 1L)) {
stop("Some columns are a multi-column type (such as a matrix column): ", brackify(which(idx)),". These cannot be converted to data.table by reference. Please use as.data.table() instead which will create a new column for each embedded column.")
}
if (is.data.table(x)) {
# fix for #1078 and #1128, see .resetclass() for explanation.
setattr(x, 'class', .resetclass(x, 'data.table'))
Expand Down
7 changes: 6 additions & 1 deletion inst/tests/tests.Rraw
Original file line number Diff line number Diff line change
Expand Up @@ -5264,7 +5264,7 @@ test(1354, X[Y, val2 := i.val2, allow.cartesian=TRUE][, val1 := NULL][order(id)]
# Fix for #475, setDT(CO2) should error, as it's trying to modify the object whose binding is locked.
# CO2 is not locked in R 2.14.1 but is in R >= 3.1.0. R NEWS isn't clear when that change happened, so just test there is an error when it is locked.
if (bindingIsLocked("CO2",as.environment("package:datasets"))) {
test(1355.1, setDT(CO2), error="Can not convert 'CO2' to data.table by reference because binding is locked.")
test(1355.1, setDT(CO2), error="Cannot convert 'CO2' to data.table by reference because binding is locked.")
} else {
test(1355.2, setDT(CO2), CO2)
}
Expand Down Expand Up @@ -15798,6 +15798,11 @@ test(2087.3, setkey(x, a, verbose=TRUE), data.table(a=paste0(1:2), b=paste0(1:2)
x = data.table(a=c(0.85, -0.38, 1.19), b=c(0.56, 0.63, -1.30))
test(2088, x[, round(.SD, 1)][, c:=8.88], data.table(a=c(.8, -.4, 1.2), b=c(.6,.6,-1.3), c=8.88))

# setDT should halt when it sees matrix columns, #3760
DF = data.frame(a=1:5)
DF$m = matrix(6:15, ncol=2L)
test(2089, setDT(DF), error='Some columns are a multi-column type.*Please use as.data.table')


###################################
# Add new tests above this line #
Expand Down

0 comments on commit 6a7246e

Please sign in to comment.