[GitHub] spark pull request #22058: [SPARK-25036][SQL][FOLLOW-UP] Avoid match may not...

2018-08-10 Thread asfgit 
Github user asfgit closed the pull request at:

https://github.com/apache/spark/pull/22058


---

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

[GitHub] spark pull request #22058: [SPARK-25036][SQL][FOLLOW-UP] Avoid match may not...

2018-08-09 Thread srowen 
Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/22058#discussion_r209027132
  
--- Diff: 
mllib/src/main/scala/org/apache/spark/ml/evaluation/ClusteringEvaluator.scala 
---
@@ -119,6 +119,8 @@ class ClusteringEvaluator @Since("2.3.0") 
(@Since("2.3.0") override val uid: Str
   df, $(predictionCol), $(featuresCol))
   case ("silhouette", "cosine") =>
 CosineSilhouette.computeSilhouetteScore(df, $(predictionCol), 
$(featuresCol))
+  case (mn, dm) =>
+throw new IllegalArgumentException(s"($mn, $dm) is not matched in 
evaluate")
--- End diff --

This is OK, but doesn't really add much beyond what the MatchError would 
have said. Worth a message like "No support for metric $mn, distance $dm"?


---

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

[GitHub] spark pull request #22058: [SPARK-25036][SQL][FOLLOW-UP] Avoid match may not...

2018-08-09 Thread kiszk 
GitHub user kiszk opened a pull request:

https://github.com/apache/spark/pull/22058

[SPARK-25036][SQL][FOLLOW-UP] Avoid match may not be exhaustive in 
Scala-2.12.

## What changes were proposed in this pull request?

This is a follow-up pr of #22014 and #22039

We still have some more compilation errors in mllib with scala-2.12 with 
sbt:

```
[error] [warn] 
/home/ishizaki/Spark/PR/scala212/spark/mllib/src/main/scala/org/apache/spark/ml/evaluation/ClusteringEvaluator.scala:116:
 match may not be exhaustive.
[error] It would fail on the following inputs: ("silhouette", _), (_, 
"cosine"), (_, "squaredEuclidean"), (_, String()), (_, _)
[error] [warn] ($(metricName), $(distanceMeasure)) match {
[error] [warn] 
```

## How was this patch tested?

Existing UTs

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/kiszk/spark SPARK-25036c

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/spark/pull/22058.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #22058


commit 6a0ee38c53d8a53d219bfec8cad9953bc9571e0c
Author: Kazuaki Ishizaki 
Date:   2018-08-09T16:29:40Z

initial commit

commit 5655d83bd2c9d6ca872c371bff421f69409b6d0b
Author: Kazuaki Ishizaki 
Date:   2018-08-09T16:45:23Z

make the change one-liner




---

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