diff --git a/NEWS.md b/NEWS.md index 7251bace79..dc66ce2897 100644 --- a/NEWS.md +++ b/NEWS.md @@ -109,6 +109,8 @@ unit = "s") 4. `fifelse` and `fcase` notify users that S4 objects (except `nanotime`) are not supported [#4135](https://github.com/Rdatatable/data.table/issues/4135). Thanks to @torema-ed for bringing it to our attention and Morgan Jacob for the PR. +5. `frank(..., ties.method="random", na.last=NA)` now returns the same random ordering that `base::rank` does, [#4243](https://github.com/Rdatatable/data.table/pull/4243). + # data.table [v1.12.8](https://github.com/Rdatatable/data.table/milestone/15?closed=1) (09 Dec 2019) diff --git a/R/frank.R b/R/frank.R index 05fd7df754..763b8267e5 100644 --- a/R/frank.R +++ b/R/frank.R @@ -32,11 +32,14 @@ frankv = function(x, cols=seq_along(x), order=1L, na.last=TRUE, ties.method=c("a nas = x[[ncol(x)]] } if (ties.method == "random") { - v = stats::runif(nrow(x)) if (is.na(na.last)) { idx = which_(nas, FALSE) - set(x, idx, '..stats_runif..', v[idx]) - } else set(x, NULL, '..stats_runif..', v) + n = length(idx) # changed in #4243 to match base R to pass test 1369 + } else { + idx = NULL + n = nrow(x) + } + set(x, idx, '..stats_runif..', stats::runif(n)) order = if (length(order) == 1L) c(rep(order, length(cols)), 1L) else c(order, 1L) cols = c(cols, ncol(x)) } diff --git a/inst/tests/tests.Rraw b/inst/tests/tests.Rraw index 2a42adf7a2..b1c080c393 100644 --- a/inst/tests/tests.Rraw +++ b/inst/tests/tests.Rraw @@ -5480,7 +5480,8 @@ test_no = 1368.0 for (i in seq_along(dt)) { col = dt[[i]] for (j in list(TRUE, FALSE, "keep")) { - for (k in c("average", "min", "max", "first", "last")) { + # ensure consistency with base::rank ties.methods as advertised + for (k in eval(formals(base::rank)$ties.method)) { if (k == "random") set.seed(45L) if (class(col) == "integer64") { r1 = rank(as.integer(col), ties.method=k, na.last=j) @@ -5511,7 +5512,8 @@ if (test_bit64) dt[, DD := as.integer64(DD)] test_no = 1369.0 for (i in seq_along(dt)) { col = dt[[i]] - for (k in c("average", "min", "max", "first", "last")) { + # ensure consistency with base::rank ties.methods as advertised + for (k in eval(formals(base::rank)$ties.method)) { # 'random' on now with tweak to match base in #4243 if (k == "random") set.seed(45L) if (class(col) == "integer64") { r1 = rank(as.integer(col), ties.method=k, na.last=NA)