From c282ec40b37075fee93a978d080f6424a419feed Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Fri, 6 Mar 2020 23:28:58 +0800 Subject: [PATCH 1/3] better error message for missing j in cube --- NEWS.md | 2 ++ R/groupingsets.R | 4 +++- inst/tests/tests.Rraw | 4 ++++ 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index 71fd76aa65..c3f86634e8 100644 --- a/NEWS.md +++ b/NEWS.md @@ -145,6 +145,8 @@ unit = "s") 7. Added more explanation/examples to `?data.table` for how to use `.BY`, [#1363](https://github.com/Rdatatable/data.table/issues/1363). +8. Cube catches a missing `j` argument earlier to give friendlier output. + # data.table [v1.12.8](https://github.com/Rdatatable/data.table/milestone/15?closed=1) (09 Dec 2019) diff --git a/R/groupingsets.R b/R/groupingsets.R index 6281615dd5..5c3ad02d4b 100644 --- a/R/groupingsets.R +++ b/R/groupingsets.R @@ -27,10 +27,12 @@ cube.data.table = function(x, j, by, .SDcols, id = FALSE, ...) { stop("Argument 'by' must be a character vector of column names used in grouping.") if (!is.logical(id)) stop("Argument 'id' must be a logical scalar.") + if (missing(j)) + stop("Argument 'j' is required") # generate grouping sets for cube - power set: http://stackoverflow.com/a/32187892/2490497 n = length(by) keepBool = sapply(2L^(seq_len(n)-1L), function(k) rep(c(FALSE, TRUE), times=k, each=((2L^n)/(2L*k)))) - sets = lapply((2L^n):1L, function(j) by[keepBool[j, ]]) + sets = lapply((2L^n):1L, function(jj) by[keepBool[jj, ]]) # redirect to workhorse function jj = substitute(j) groupingsets.data.table(x, by=by, sets=sets, .SDcols=.SDcols, id=id, jj=jj) diff --git a/inst/tests/tests.Rraw b/inst/tests/tests.Rraw index 7cc6819e8f..123a763a81 100644 --- a/inst/tests/tests.Rraw +++ b/inst/tests/tests.Rraw @@ -16846,3 +16846,7 @@ A = data.table(A=c(complex(real = 1:3, imaginary=c(0, -1, 1)), NaN)) test(2138.3, rbind(A,B), data.table(A=c(as.character(A$A), B$A))) A = data.table(A=as.complex(rep(NA, 5))) test(2138.4, rbind(A,B), data.table(A=c(as.character(A$A), B$A))) + +# missing j was only caught in groupingsets, leading to unexpected error message +DT = data.table(a = 1) +test(2139, cube(DT, by = 'a'), error = "Argument 'j' is required") From 4e29ada4bcd11343f8da36ca10299af7aaafefe4 Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Sat, 7 Mar 2020 16:39:44 +0800 Subject: [PATCH 2/3] fixed, thanks jan --- NEWS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index c3f86634e8..bfc3f1cc80 100644 --- a/NEWS.md +++ b/NEWS.md @@ -145,7 +145,7 @@ unit = "s") 7. Added more explanation/examples to `?data.table` for how to use `.BY`, [#1363](https://github.com/Rdatatable/data.table/issues/1363). -8. Cube catches a missing `j` argument earlier to give friendlier output. +8. The `data.table` method for `cube` catches a missing `j` argument earlier to give friendlier output. # data.table [v1.12.8](https://github.com/Rdatatable/data.table/milestone/15?closed=1) (09 Dec 2019) From 1e9c8ec3abecb08dcda6d23912934c9b3faef444 Mon Sep 17 00:00:00 2001 From: Matt Dowle Date: Wed, 14 Apr 2021 23:31:31 -0600 Subject: [PATCH 3/3] news item: move subject (cube) to be first word --- NEWS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index cc229084e3..12c05d4dff 100644 --- a/NEWS.md +++ b/NEWS.md @@ -30,7 +30,7 @@ 3. In v1.12.4, fractional `fread(..., stringsAsFactors=)` was added. For example if `stringsAsFactors=0.2`, any character column with fewer than 20% unique strings would be cast as `factor`. This is now documented in `?fread` as well, [#4706](https://github.com/Rdatatable/data.table/issues/4706). Thanks to @markderry for the PR. -4. `cube(DT)` now catches a missing `j` argument earlier to give friendlier output. +4. `cube(DT, by="a")` now gives a more helpful error that `j` is missing, [#4282](https://github.com/Rdatatable/data.table/pull/4282). # data.table [v1.14.0](https://github.com/Rdatatable/data.table/milestone/23?closed=1) (21 Feb 2021)