[SPARK-12044] [SparkR] Fix usage of isnan, isNaN #10037
Conversation
|
Test build #46874 has finished for PR 10037 at commit
|
R/pkg/inst/tests/test_sparkSQL.R
Outdated
There was a problem hiding this comment.
please add test cases for "isNaN", "isNull", "isNotNul" for Column
|
LGTM except that it's good to add additional test cases. |
R/pkg/R/functions.R
Outdated
There was a problem hiding this comment.
should this be called (or have an alias) is.nan?
https://stat.ethz.ch/R-manual/R-devel/library/base/html/is.finite.html
There was a problem hiding this comment.
yes, it makes sense to add an alias as is.nan
|
Test build #47058 has finished for PR 10037 at commit
|
|
@yanboliang, I am ok that we leave alias of is.na to SPARK-12071. But it would be better you can add alias is.nan. |
|
Test build #47065 has finished for PR 10037 at commit
|
R/pkg/R/generics.R
Outdated
There was a problem hiding this comment.
it seems that "is.nan" in base package is masked? There is an implicit generic function for a primitive function. I think no need to define it here.
There was a problem hiding this comment.
OK, I will update it.
There was a problem hiding this comment.
please also add a test for base::is.nan if we are masking it. See https://github.com/yanboliang/spark/blob/spark-12044/R/pkg/inst/tests/test_sparkSQL.R#L931
|
Test build #47123 has finished for PR 10037 at commit
|
|
LGTM |
|
Test build #47127 has finished for PR 10037 at commit
|
yanboliang
changed the title
yanboliang
changed the title
|
Thanks @sun-rui @felixcheung for the clarifications and @yanboliang for the change. LGTM. Merging this to master, branch-1.6 |
1, Add ```isNaN``` to ```Column``` for SparkR. ```Column``` should has three related variable functions: ```isNaN, isNull, isNotNull```. 2, Replace ```DataFrame.isNaN``` with ```DataFrame.isnan``` at SparkR side. Because ```DataFrame.isNaN``` has been deprecated and will be removed at Spark 2.0. <del>3, Add ```isnull``` to ```DataFrame``` for SparkR. ```DataFrame``` should has two related functions: ```isnan, isnull```.<del> cc shivaram sun-rui felixcheung Author: Yanbo Liang <[email protected]> Closes #10037 from yanboliang/spark-12044. (cherry picked from commit b6e8e63) Signed-off-by: Shivaram Venkataraman <[email protected]>
|
Thanks for your help @sun-rui @felixcheung @shivaram . |
5 participants
1, Add
isNaNtoColumnfor SparkR.Columnshould has three related variable functions:isNaN, isNull, isNotNull.2, Replace
DataFrame.isNaNwithDataFrame.isnanat SparkR side. BecauseDataFrame.isNaNhas been deprecated and will be removed at Spark 2.0.3, AddisnulltoDataFramefor SparkR.DataFrameshould has two related functions:isnan, isnull.cc @shivaram @sun-rui @felixcheung