Skip to content

order optimizes to forderv, uses key or index, forder more consistent to base order#4458

Draft
jangorecki wants to merge 14 commits intomasterfrom
opt-order
Draft

order optimizes to forderv, uses key or index, forder more consistent to base order#4458
jangorecki wants to merge 14 commits intomasterfrom
opt-order

Conversation

@jangorecki
Copy link
Copy Markdown
Member

@jangorecki jangorecki commented May 18, 2020

Closes:

To reduce number of API changes (in still internal forder[v]) I am not going to yet close this issue:

It is partially addressed by making forder compatible with base::order, but this PR does not restrict input types. Lets see what base R is planning to do about order(df) (see R-devel discussion), and in another iteration we can solve #4346 and #4214.

It also makes na.last=NA option in forder consistent to base order by removing 0's that are result from forderv(na.last=NA).

@codecov
Copy link
Copy Markdown

codecov bot commented May 18, 2020

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.59%. Comparing base (dd7609e) to head (7791b6e).
⚠️ Report is 1928 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4458      +/-   ##
==========================================
- Coverage   99.61%   99.59%   -0.02%     
==========================================
  Files          72       72              
  Lines       13917    13967      +50     
==========================================
+ Hits        13863    13911      +48     
- Misses         54       56       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread R/setkey.R
o
if (!is.logical(decreasing) || anyNA(decreasing)) stop("'decreasing' must be logical non-NA")
if (length(decreasing)!=1L && length(decreasing)!=length(data)) stop("'decreasing' must be either length 1, or length of the variables passed to [f]order")
asc[decreasing] = -(asc[decreasing])
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[.integer automatically recycle scalar decreasing

@jangorecki jangorecki removed the WIP label May 18, 2020
@jangorecki jangorecki added the WIP label May 19, 2020
@jangorecki
Copy link
Copy Markdown
Member Author

marking as WIP because it will need rebase over lazy-forder branch

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

DT[order(.), ...] could create index on DT order could utilize existing index order optimization does not recognize all order args properly

3 participants