Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Also switch to forder on subsetted ordering in i #1921

MichaelChirico opened this issue Nov 20, 2016 · 1 comment · Fixed by #3604

Also switch to forder on subsetted ordering in i #1921

MichaelChirico opened this issue Nov 20, 2016 · 1 comment · Fixed by #3604


Copy link

@MichaelChirico MichaelChirico commented Nov 20, 2016

If we want to get the top five entries of a column, I guess the canonical approach is:

DT = data.table(A=sample(3, 10, TRUE), 
         B=sample(letters[1:3], 10, TRUE), C=sample(10))

DT[order(C), C[1:5]]

But I've found in more general scenarios (for example, subsetting on the top five of C then performing some operations on other columns) the following approach to be useful :

DT[order(C)[1:5], B]

However, this approach kills forder detection:

DT[order(C)[1:5], B, verbose = TRUE]

I guess it's easy to detect this and do forder and then subset.

@jangorecki jangorecki added this to the 1.12.4 milestone Mar 5, 2019
Copy link

@jangorecki jangorecki commented Mar 5, 2019


Lines 430 to 441 in 0bf0e33

if ( && isub[[1L]] =="order") && getOption("datatable.optimize") >= 1) { # optimize here so that we can switch it off if needed
if (verbose) cat("order optimisation is on, i changed from 'order(...)' to 'forder(DT, ...)'.\n")
isub = as.list(isub)
isub =, quote(x)), isub[-1L]))
if (is.null(isub)) return( )
if ( && isub[[1L]] == quote(forder)) {
order_env = new.env(parent=parent.frame()) # until 'forder' is exported
assign("forder", forder, order_env)
assign("x", x, order_env)
i = eval(isub, order_env, parent.frame()) # for optimisation of 'order' to 'forder'
# that forder returns empty integer() is taken care of internally within forder

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
None yet
None yet

Successfully merging a pull request may close this issue.

2 participants