[SPARK-19682][SparkR] Issue warning (or error) when subset method "[[" takes vector index #17017
Conversation
|
@felixcheung Instead of issuing warning and taking the first element, we can throw a more meaningful error message. Minor issue, but many R users may be accustomed to use vector index with Another possibility is to return a dataframe with the selected columns, e.g., |
|
Test build #73227 has finished for PR 17017 at commit
|
|
Test build #73228 has finished for PR 17017 at commit
|
|
Test build #73231 has finished for PR 17017 at commit
|
|
Jenkins, retest this please |
|
That's one weird test failure. |
|
Test build #73264 has finished for PR 17017 at commit
|
| if (length(i) > 1) { | ||
| warning("Subset index has length > 1. Only the first index is used.") | ||
| i <- i[1] | ||
| } |
There was a problem hiding this comment.
actually, sorry I missed this - could you add a test for this?
There was a problem hiding this comment.
@felixcheung Test added. Thanks for catching this.
|
Test build #73329 has finished for PR 17017 at commit
|
…" takes vector index ## What changes were proposed in this pull request? The `[[` method is supposed to take a single index and return a column. This is different from base R which takes a vector index. We should check for this and issue warning or error when vector index is supplied (which is very likely given the behavior in base R). Currently I'm issuing a warning message and just take the first element of the vector index. We could change this to an error it that's better. ## How was this patch tested? new tests Author: actuaryzhang <[email protected]> Closes #17017 from actuaryzhang/sparkRSubsetter. (cherry picked from commit 7bf0943) Signed-off-by: Felix Cheung <[email protected]>
|
thanks - merged to master and branch-2.1 |
…" takes vector index ## What changes were proposed in this pull request? The `[[` method is supposed to take a single index and return a column. This is different from base R which takes a vector index. We should check for this and issue warning or error when vector index is supplied (which is very likely given the behavior in base R). Currently I'm issuing a warning message and just take the first element of the vector index. We could change this to an error it that's better. ## How was this patch tested? new tests Author: actuaryzhang <[email protected]> Closes apache#17017 from actuaryzhang/sparkRSubsetter.
3 participants
What changes were proposed in this pull request?
The
[[method is supposed to take a single index and return a column. This is different from base R which takes a vector index. We should check for this and issue warning or error when vector index is supplied (which is very likely given the behavior in base R).Currently I'm issuing a warning message and just take the first element of the vector index. We could change this to an error it that's better.
How was this patch tested?
new tests