-
Notifications
You must be signed in to change notification settings - Fork 1k
export is.sorted #4373
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
base: master
Are you sure you want to change the base?
export is.sorted #4373
Conversation
add argument retOrd to return order unfold C for codecov
| } else if (is.null(x)) { # NULL does not satisfy C isVectorAtomic | ||
| NA | ||
| } else if (is.atomic(x)) { | ||
| if (isTRUE(retOrd)) stop("retOrd works only for data.table/list input") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I saw this error message, I would make my vector a list. Should we automatically do list(x) for convenience?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On one hand it would be nice, but fsorted that we use for atomic vectors is likely to be much more efficient. So by default it definitely not good to wrap it in list.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only wrap it if retOrd == TRUE. That arg would seem to trump performance considerations of fsorted.
| } | ||
| \arguments{ | ||
| \item{x}{ data.table type object or atomic vector. } | ||
| \item{by}{ data.table columns used to check if \code{x} is sorted by those columns. } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
by = seq_along(x) implies an integer argument. Since a character vector is supported, we may want to point it out.
|
@ColeMiller1 maybe I should just export |
| \item{retOrd}{ logical, when \code{TRUE} it will set an attribute \code{"order"} on the returned value, providing an order of \code{x}. Works only for data.table type \code{x}, not for atomic vector. } | ||
| } | ||
| \details{ | ||
| Checks if the input is object is sorted. Can check also by a subset of columns provided in \code{by} argument. Can also return an order used in computation when using \code{retOrd} argument. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should possibly mention integer() trick in case for 1:n
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is actually explained later on, so maybe just a pointer to section in the manual
|
Argument retOrd is overlapping with #3447 and could possibly be removed from this PR |
add argument retOrd to return order
unfold C for codecov
closes #2325