From dbb77942ecafc0f134ccaa65914c8bf9f488c7df Mon Sep 17 00:00:00 2001 From: venom1204 Date: Mon, 20 Jan 2025 23:55:03 +0530 Subject: [PATCH 1/9] checking RHS of := --- R/data.table.R | 3 +++ 1 file changed, 3 insertions(+) diff --git a/R/data.table.R b/R/data.table.R index e5feb0c566..cfd22c6b84 100644 --- a/R/data.table.R +++ b/R/data.table.R @@ -316,6 +316,9 @@ replace_dot_alias = function(e) { warningf("nomatch isn't relevant together with :=, ignoring nomatch") nomatch=NULL } + if (length(jsub) >= 3L && is.function(jsub[[3L]])) { + stopf("the right hand side of := should be a vector, list, data.frame, function or call, not a function,to store a function wrap it in a list: DT[, newcol := list(myfun)]") + } } } From 49c52d075d7fdd8d12224592705c1c4e0f25f95b Mon Sep 17 00:00:00 2001 From: venom1204 Date: Fri, 6 Jun 2025 18:44:53 +0000 Subject: [PATCH 2/9] added condition to check the rhs --- R/data.table.R | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/R/data.table.R b/R/data.table.R index fae98c3ac9..606debcb7f 100644 --- a/R/data.table.R +++ b/R/data.table.R @@ -1403,6 +1403,16 @@ replace_dot_alias = function(e) { } if (!is.null(lhs)) { + # We only need to check if we are creating new columns. + newnames = setdiff(lhs, names(x)) + + if (length(newnames) > 0) { + if (is.function(jval)) { + stopf("RHS of `:=` is a function. To create a new column of functions, it must be a list column (e.g., wrap in `list()`).") + } else if (is.list(jval) && length(jval) == 1 && is.function(jval[[1L]])) { + stopf("RHS of `:=` is a length-1 list containing a function. `data.table` does not automatically recycle lists. To create a list-column, use `rep(list(myfun), .N)` or `list(replicate(.N, myfun))` to match the number of rows.") + } + } # TODO?: use set() here now that it can add new columns. Then remove newnames and alloc logic above. .Call(Cassign,x,irows,cols,newnames,jval) return(suppPrint(x)) From 1d29473759a8e115c45c83cb90dafccad6a1c3ed Mon Sep 17 00:00:00 2001 From: venom1204 Date: Fri, 6 Jun 2025 20:21:13 +0000 Subject: [PATCH 3/9] added tests --- R/data.table.R | 2 +- inst/tests/tests.Rraw | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/R/data.table.R b/R/data.table.R index 606debcb7f..c8d6267c82 100644 --- a/R/data.table.R +++ b/R/data.table.R @@ -1410,7 +1410,7 @@ replace_dot_alias = function(e) { if (is.function(jval)) { stopf("RHS of `:=` is a function. To create a new column of functions, it must be a list column (e.g., wrap in `list()`).") } else if (is.list(jval) && length(jval) == 1 && is.function(jval[[1L]])) { - stopf("RHS of `:=` is a length-1 list containing a function. `data.table` does not automatically recycle lists. To create a list-column, use `rep(list(myfun), .N)` or `list(replicate(.N, myfun))` to match the number of rows.") + stopf("RHS of `:=` is a length-1 list containing a function. `data.table` does not automatically recycle lists. To create a list-column, use `rep(list(myfun), .N)` to match the number of rows.") } } # TODO?: use set() here now that it can add new columns. Then remove newnames and alloc logic above. diff --git a/inst/tests/tests.Rraw b/inst/tests/tests.Rraw index c9cfe6244b..f723b857c1 100644 --- a/inst/tests/tests.Rraw +++ b/inst/tests/tests.Rraw @@ -21157,3 +21157,8 @@ test(2317.6, DT1[DF1, on='a', .(d = x.a + i.d)]$d, 5) test(2317.7, DT1[DF2, on='a', e := i.e]$e, 5) test(2317.8, DT1[DF2, on='a', e2 := x.a + i.e]$e2, 6) test(2317.9, DT1[DF2, on='a', .(e = x.a + i.e)]$e, 6) + +DT = data.table(x = 1:3) + +test(2318.1, DT[, y := mean], error = "RHS of `:=` is a function") +test(2318.2, DT[, y := list(mean)], error = "RHS of `:=` is a length-1 list containing a function") From 5f7f5ff644193670a67e79dcba8907722bfa6bfc Mon Sep 17 00:00:00 2001 From: venom1204 Date: Fri, 6 Jun 2025 20:29:11 +0000 Subject: [PATCH 4/9] k --- R/data.table.R | 2 +- inst/tests/tests.Rraw | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/R/data.table.R b/R/data.table.R index c8d6267c82..eba9554c26 100644 --- a/R/data.table.R +++ b/R/data.table.R @@ -1413,7 +1413,7 @@ replace_dot_alias = function(e) { stopf("RHS of `:=` is a length-1 list containing a function. `data.table` does not automatically recycle lists. To create a list-column, use `rep(list(myfun), .N)` to match the number of rows.") } } - # TODO?: use set() here now that it can add new columns. Then remove newnames and alloc logic above. + # TODO?: use set() here now that it can add new columns.Then remove newnames and alloc logic above. .Call(Cassign,x,irows,cols,newnames,jval) return(suppPrint(x)) } diff --git a/inst/tests/tests.Rraw b/inst/tests/tests.Rraw index f723b857c1..6a313d84d0 100644 --- a/inst/tests/tests.Rraw +++ b/inst/tests/tests.Rraw @@ -21161,4 +21161,4 @@ test(2317.9, DT1[DF2, on='a', .(e = x.a + i.e)]$e, 6) DT = data.table(x = 1:3) test(2318.1, DT[, y := mean], error = "RHS of `:=` is a function") -test(2318.2, DT[, y := list(mean)], error = "RHS of `:=` is a length-1 list containing a function") +test(2318.2, DT[, y := list(mean)], error = "RHS of `:=` is a length-1 list containing a function") \ No newline at end of file From c9693bdb980451711cef38823abecf940fbdaffc Mon Sep 17 00:00:00 2001 From: venom1204 Date: Fri, 6 Jun 2025 20:54:24 +0000 Subject: [PATCH 5/9] merged branch --- R/data.table.R | 3 --- 1 file changed, 3 deletions(-) diff --git a/R/data.table.R b/R/data.table.R index e78210a2f0..eba9554c26 100644 --- a/R/data.table.R +++ b/R/data.table.R @@ -316,9 +316,6 @@ replace_dot_alias = function(e) { warningf("nomatch isn't relevant together with :=, ignoring nomatch") nomatch=NULL } - if (length(jsub) >= 3L && is.function(jsub[[3L]])) { - stopf("the right hand side of := should be a vector, list, data.frame, function or call, not a function,to store a function wrap it in a list: DT[, newcol := list(myfun)]") - } } } From 2dfd231afe2845969b3a13f550844835c85c7013 Mon Sep 17 00:00:00 2001 From: venom1204 Date: Fri, 6 Jun 2025 21:11:15 +0000 Subject: [PATCH 6/9] lint r --- R/data.table.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/data.table.R b/R/data.table.R index eba9554c26..3fee35aa9b 100644 --- a/R/data.table.R +++ b/R/data.table.R @@ -1407,7 +1407,7 @@ replace_dot_alias = function(e) { newnames = setdiff(lhs, names(x)) if (length(newnames) > 0) { - if (is.function(jval)) { + if (is.function(jval)) { stopf("RHS of `:=` is a function. To create a new column of functions, it must be a list column (e.g., wrap in `list()`).") } else if (is.list(jval) && length(jval) == 1 && is.function(jval[[1L]])) { stopf("RHS of `:=` is a length-1 list containing a function. `data.table` does not automatically recycle lists. To create a list-column, use `rep(list(myfun), .N)` to match the number of rows.") From 2ded81a586f9f0db62ba7adb96f6119ad74c6b09 Mon Sep 17 00:00:00 2001 From: venom1204 Date: Fri, 6 Jun 2025 21:18:48 +0000 Subject: [PATCH 7/9] lintr --- R/data.table.R | 1 - 1 file changed, 1 deletion(-) diff --git a/R/data.table.R b/R/data.table.R index 3fee35aa9b..12768a5687 100644 --- a/R/data.table.R +++ b/R/data.table.R @@ -1405,7 +1405,6 @@ replace_dot_alias = function(e) { if (!is.null(lhs)) { # We only need to check if we are creating new columns. newnames = setdiff(lhs, names(x)) - if (length(newnames) > 0) { if (is.function(jval)) { stopf("RHS of `:=` is a function. To create a new column of functions, it must be a list column (e.g., wrap in `list()`).") From e50316ae22c554627cbe192c5140e419dd19daef Mon Sep 17 00:00:00 2001 From: venom1204 Date: Sun, 8 Jun 2025 20:25:34 +0000 Subject: [PATCH 8/9] updated --- R/data.table.R | 17 ++++++++++------- inst/tests/tests.Rraw | 7 ++++++- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/R/data.table.R b/R/data.table.R index 12768a5687..a3c34d5612 100644 --- a/R/data.table.R +++ b/R/data.table.R @@ -1403,15 +1403,18 @@ replace_dot_alias = function(e) { } if (!is.null(lhs)) { - # We only need to check if we are creating new columns. - newnames = setdiff(lhs, names(x)) - if (length(newnames) > 0) { - if (is.function(jval)) { - stopf("RHS of `:=` is a function. To create a new column of functions, it must be a list column (e.g., wrap in `list()`).") - } else if (is.list(jval) && length(jval) == 1 && is.function(jval[[1L]])) { - stopf("RHS of `:=` is a length-1 list containing a function. `data.table` does not automatically recycle lists. To create a list-column, use `rep(list(myfun), .N)` to match the number of rows.") + if (length(cols) == 1L) { + is_new_col <- cols > ncol(x) + if (is_new_col || !is.list(x[[cols]])) { + if (is.function(jval)) { + stopf("RHS of `:=` is a function. To create a new column of functions or assign to a non-list-column, it must be wrapped in a list(), e.g., `:= list(myfun)`.") + } + if (is.list(jval) && length(jval) == 1L && is.function(jval[[1L]]) && nrow(x) > 1L) { + stopf("RHS of `:=` is a length-1 list containing a function. `data.table` does not automatically recycle lists. To create a list-column, use `rep(list(myfun), .N)`.") + } } } + newnames = setdiff(lhs, names(x)) # TODO?: use set() here now that it can add new columns.Then remove newnames and alloc logic above. .Call(Cassign,x,irows,cols,newnames,jval) return(suppPrint(x)) diff --git a/inst/tests/tests.Rraw b/inst/tests/tests.Rraw index 6a313d84d0..cec66452ca 100644 --- a/inst/tests/tests.Rraw +++ b/inst/tests/tests.Rraw @@ -21161,4 +21161,9 @@ test(2317.9, DT1[DF2, on='a', .(e = x.a + i.e)]$e, 6) DT = data.table(x = 1:3) test(2318.1, DT[, y := mean], error = "RHS of `:=` is a function") -test(2318.2, DT[, y := list(mean)], error = "RHS of `:=` is a length-1 list containing a function") \ No newline at end of file +test(2318.2, DT[, y := list(mean)], error = "RHS of `:=` is a length-1 list containing a function") +f = mean +test(2318.3, DT[, y := f], error = "RHS of `:=` is a function") +test(2318.4, DT[, y := identity(mean)], error = "RHS of `:=` is a function") +test(2318.5, data.table(x = 1)[, y := mean], error = "RHS of `:=` is a function") +test(2318.6, data.table(x = 1:2)[, y := mean], error = "RHS of `:=` is a function") From b295e9b24c3bcebb33b89ece55d0bf270f897781 Mon Sep 17 00:00:00 2001 From: venom1204 Date: Tue, 10 Jun 2025 19:00:47 +0000 Subject: [PATCH 9/9] removed the list check --- R/data.table.R | 15 +++++---------- inst/tests/tests.Rraw | 7 +------ 2 files changed, 6 insertions(+), 16 deletions(-) diff --git a/R/data.table.R b/R/data.table.R index a3c34d5612..69251c9a81 100644 --- a/R/data.table.R +++ b/R/data.table.R @@ -1403,18 +1403,13 @@ replace_dot_alias = function(e) { } if (!is.null(lhs)) { - if (length(cols) == 1L) { - is_new_col <- cols > ncol(x) - if (is_new_col || !is.list(x[[cols]])) { - if (is.function(jval)) { - stopf("RHS of `:=` is a function. To create a new column of functions or assign to a non-list-column, it must be wrapped in a list(), e.g., `:= list(myfun)`.") - } - if (is.list(jval) && length(jval) == 1L && is.function(jval[[1L]]) && nrow(x) > 1L) { - stopf("RHS of `:=` is a length-1 list containing a function. `data.table` does not automatically recycle lists. To create a list-column, use `rep(list(myfun), .N)`.") - } + newnames = setdiff(lhs, names(x)) + + if (length(newnames) > 0) { + if (is.function(jval)) { + stopf("RHS of `:=` is a function. To create a new column of functions, it must be a list column (e.g., wrap the function in `list()`).") } } - newnames = setdiff(lhs, names(x)) # TODO?: use set() here now that it can add new columns.Then remove newnames and alloc logic above. .Call(Cassign,x,irows,cols,newnames,jval) return(suppPrint(x)) diff --git a/inst/tests/tests.Rraw b/inst/tests/tests.Rraw index cec66452ca..28eee66dfc 100644 --- a/inst/tests/tests.Rraw +++ b/inst/tests/tests.Rraw @@ -21161,9 +21161,4 @@ test(2317.9, DT1[DF2, on='a', .(e = x.a + i.e)]$e, 6) DT = data.table(x = 1:3) test(2318.1, DT[, y := mean], error = "RHS of `:=` is a function") -test(2318.2, DT[, y := list(mean)], error = "RHS of `:=` is a length-1 list containing a function") -f = mean -test(2318.3, DT[, y := f], error = "RHS of `:=` is a function") -test(2318.4, DT[, y := identity(mean)], error = "RHS of `:=` is a function") -test(2318.5, data.table(x = 1)[, y := mean], error = "RHS of `:=` is a function") -test(2318.6, data.table(x = 1:2)[, y := mean], error = "RHS of `:=` is a function") +test(2318.2, DT[, y := function(x) x], error = "RHS of `:=` is a function") \ No newline at end of file