Repository: spark
Updated Branches:
  refs/heads/master c133907c5 -> 6902edab7


[SPARK-17315][FOLLOW-UP][SPARKR][ML] Fix print of Kolmogorov-Smirnov test 
summary

## What changes were proposed in this pull request?
#14881 added Kolmogorov-Smirnov Test wrapper to SparkR. I found that 
```print.summary.KSTest``` was implemented inappropriately and result in no 
effect.
Running the following code for KSTest:
```Scala
data <- data.frame(test = c(0.1, 0.15, 0.2, 0.3, 0.25, -1, -0.5))
df <- createDataFrame(data)
testResult <- spark.kstest(df, "test", "norm")
summary(testResult)
```
Before this PR:
![image](https://cloud.githubusercontent.com/assets/1962026/18615016/b9a2823a-7d4f-11e6-934b-128beade355e.png)
After this PR:
![image](https://cloud.githubusercontent.com/assets/1962026/18615014/aafe2798-7d4f-11e6-8b99-c705bb9fe8f2.png)
The new implementation is similar with 
[```print.summary.GeneralizedLinearRegressionModel```](https://github.com/apache/spark/blob/master/R/pkg/R/mllib.R#L284)
 of SparkR and 
[```print.summary.glm```](https://svn.r-project.org/R/trunk/src/library/stats/R/glm.R)
 of native R.

BTW, I removed the comparison of ```print.summary.KSTest``` in unit test, since 
it's only wrappers of the summary output which has been checked. Another reason 
is that these comparison will output summary information to the test console, 
it will make the test output in a mess.

## How was this patch tested?
Existing test.

Author: Yanbo Liang <yblia...@gmail.com>

Closes #15139 from yanboliang/spark-17315.


Project: http://git-wip-us.apache.org/repos/asf/spark/repo
Commit: http://git-wip-us.apache.org/repos/asf/spark/commit/6902edab
Tree: http://git-wip-us.apache.org/repos/asf/spark/tree/6902edab
Diff: http://git-wip-us.apache.org/repos/asf/spark/diff/6902edab

Branch: refs/heads/master
Commit: 6902edab7e80e96e3f57cf80f26cefb209d4d63c
Parents: c133907
Author: Yanbo Liang <yblia...@gmail.com>
Authored: Wed Sep 21 20:14:18 2016 -0700
Committer: Yanbo Liang <yblia...@gmail.com>
Committed: Wed Sep 21 20:14:18 2016 -0700

----------------------------------------------------------------------
 R/pkg/R/mllib.R                        | 16 +++++++++-------
 R/pkg/inst/tests/testthat/test_mllib.R | 16 ++--------------
 2 files changed, 11 insertions(+), 21 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/spark/blob/6902edab/R/pkg/R/mllib.R
----------------------------------------------------------------------
diff --git a/R/pkg/R/mllib.R b/R/pkg/R/mllib.R
index 234b208..98db367 100644
--- a/R/pkg/R/mllib.R
+++ b/R/pkg/R/mllib.R
@@ -1398,20 +1398,22 @@ setMethod("summary", signature(object = "KSTest"),
             distParams <- unlist(callJMethod(jobj, "distParams"))
             degreesOfFreedom <- callJMethod(jobj, "degreesOfFreedom")
 
-            list(p.value = pValue, statistic = statistic, nullHypothesis = 
nullHypothesis,
-                 nullHypothesis.name = distName, nullHypothesis.parameters = 
distParams,
-                 degreesOfFreedom = degreesOfFreedom)
+            ans <- list(p.value = pValue, statistic = statistic, 
nullHypothesis = nullHypothesis,
+                        nullHypothesis.name = distName, 
nullHypothesis.parameters = distParams,
+                        degreesOfFreedom = degreesOfFreedom, jobj = jobj)
+            class(ans) <- "summary.KSTest"
+            ans
           })
 
 #  Prints the summary of KSTest
 
 #' @rdname spark.kstest
-#' @param x test result object of KSTest by \code{spark.kstest}.
+#' @param x summary object of KSTest returned by \code{summary}.
 #' @export
 #' @note print.summary.KSTest since 2.1.0
 print.summary.KSTest <- function(x, ...) {
-  jobj <- x@jobj
+  jobj <- x$jobj
   summaryStr <- callJMethod(jobj, "summary")
-  cat(summaryStr)
-  invisible(summaryStr)
+  cat(summaryStr, "\n")
+  invisible(x)
 }

http://git-wip-us.apache.org/repos/asf/spark/blob/6902edab/R/pkg/inst/tests/testthat/test_mllib.R
----------------------------------------------------------------------
diff --git a/R/pkg/inst/tests/testthat/test_mllib.R 
b/R/pkg/inst/tests/testthat/test_mllib.R
index 5b1404c..24c40a8 100644
--- a/R/pkg/inst/tests/testthat/test_mllib.R
+++ b/R/pkg/inst/tests/testthat/test_mllib.R
@@ -760,13 +760,7 @@ test_that("spark.kstest", {
 
   expect_equal(stats$p.value, rStats$p.value, tolerance = 1e-4)
   expect_equal(stats$statistic, unname(rStats$statistic), tolerance = 1e-4)
-
-  printStr <- print.summary.KSTest(testResult)
-  expect_match(printStr, paste0("Kolmogorov-Smirnov test summary:\\n",
-                                "degrees of freedom = 0 \\n",
-                                "statistic = 0.38208[0-9]* \\n",
-                                "pValue = 0.19849[0-9]* \\n",
-                                ".*"), perl = TRUE)
+  expect_match(capture.output(stats)[1], "Kolmogorov-Smirnov test summary:")
 
   testResult <- spark.kstest(df, "test", "norm", -0.5)
   stats <- summary(testResult)
@@ -775,13 +769,7 @@ test_that("spark.kstest", {
 
   expect_equal(stats$p.value, rStats$p.value, tolerance = 1e-4)
   expect_equal(stats$statistic, unname(rStats$statistic), tolerance = 1e-4)
-
-  printStr <- print.summary.KSTest(testResult)
-  expect_match(printStr, paste0("Kolmogorov-Smirnov test summary:\\n",
-                                "degrees of freedom = 0 \\n",
-                                "statistic = 0.44003[0-9]* \\n",
-                                "pValue = 0.09470[0-9]* \\n",
-                                ".*"), perl = TRUE)
+  expect_match(capture.output(stats)[1], "Kolmogorov-Smirnov test summary:")
 })
 
 sparkR.session.stop()


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@spark.apache.org
For additional commands, e-mail: commits-h...@spark.apache.org

Reply via email to