[SPARK-9316] [SPARKR] Add support for filtering using [ (synonym for filter / select)
#8394
Conversation
|
Jenkins, ok to test |
|
Test build #41458 has finished for PR 8394 at commit
|
| @@ -945,6 +947,19 @@ setMethod("[", signature(x = "DataFrame", i = "missing"), | |||
| select(x, j) | |||
| }) | |||
|
|
|||
| #' @rdname select | |||
| setMethod("[", signature(x = "DataFrame", i = "Column"), | |||
| function(x, i, j, ...) { | |||
There was a problem hiding this comment.
... seems not necessary? It allows user pass parameters that won't be used, which may be confusing.
There was a problem hiding this comment.
Are you referring to ...?
You are probably right... I'm more or less following line 938 above - since this is a i-not-missing overload of the operator/method.
setMethod("[", signature(x = "DataFrame", i = "missing"),
function(x, i, j, ...) {
setMethod("[", signature(x = "DataFrame", i = "Column"),
function(x, i, j, ...) {
Perhaps we should take both out? But that would be a breaking change.
There was a problem hiding this comment.
So we need the ... to match the signature of [ in base R. From the help page you can see that the signatures look like
x[i]
x[i, j, ... , drop = TRUE]
x[[i, exact = TRUE]]
x[[i, j, ..., exact = TRUE]]
There was a problem hiding this comment.
Also FYI the Matrix package which also uses S4 is a good example for things like this
https://github.com/rforge/matrix/blob/master/pkg/Matrix/R/Matrix.R#L457
|
LGTM. some minor comments. |
|
Test build #41517 has finished for PR 8394 at commit
|
|
Thanks @felixcheung -- this LGTM to me as well |
Conflicts: R/pkg/R/DataFrame.R
|
Test build #41599 has finished for PR 8394 at commit
|
|
Thanks @felixcheung -- Merging this |
…r filter / select) Add support for ``` df[df$name == "Smith", c(1,2)] df[df$age %in% c(19, 30), 1:2] ``` shivaram Author: felixcheung <felixcheung_m@hotmail.com> Closes #8394 from felixcheung/rsubset. (cherry picked from commit 75d4773) Signed-off-by: Shivaram Venkataraman <shivaram@cs.berkeley.edu>
Add support for
@shivaram