[GitHub] spark issue #19599: [SPARK-22381] [ML] Add StringParam that supports valid o...
Github user hhbyyh commented on the issue: https://github.com/apache/spark/pull/19599 Please advice if this is a good feature to add. If not I'll close it. Thanks. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #19599: [SPARK-22381] [ML] Add StringParam that supports valid o...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/19599 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/84887/ Test PASSed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #19599: [SPARK-22381] [ML] Add StringParam that supports valid o...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/19599 Merged build finished. Test PASSed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #19599: [SPARK-22381] [ML] Add StringParam that supports valid o...
Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/19599 **[Test build #84887 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84887/testReport)** for PR 19599 at commit [`b79d8db`](https://github.com/apache/spark/commit/b79d8db9406fbd29ef46c8a74f8591d2aace45ee). * This patch passes all tests. * This patch merges cleanly. * This patch adds no public classes. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #19599: [SPARK-22381] [ML] Add StringParam that supports valid o...
Github user hhbyyh commented on the issue: https://github.com/apache/spark/pull/19599 Updated, use $lc and add a new unit test for doc and exception. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #19599: [SPARK-22381] [ML] Add StringParam that supports valid o...
Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/19599 **[Test build #84887 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84887/testReport)** for PR 19599 at commit [`b79d8db`](https://github.com/apache/spark/commit/b79d8db9406fbd29ef46c8a74f8591d2aace45ee). --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #19599: [SPARK-22381] [ML] Add StringParam that supports valid o...
Github user hhbyyh commented on the issue: https://github.com/apache/spark/pull/19599 > One option that came to my mind was that $ returns lowercase, so this is used at most places but when you really need it you can access the original (not necessarily lowercase) value. As in the previous commits of this PR, I tried to add a new `$lc` function in trait `Params`, which works with Param[String] and return the lowercased version. But that breaks the binary compatibility and I guess that's the bigger evil. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #19599: [SPARK-22381] [ML] Add StringParam that supports valid o...
Github user hhbyyh commented on the issue: https://github.com/apache/spark/pull/19599 Many thanks for the review @smurakozi and @attilapiros. > The PR is not complete (did not convert all Param[String] instances to StringParam consistently) so it should be marked as WIP. StringParam is compatible with existing Param[String] so we don't need to update all the Param[String] together. Usually I would wait for some time to confirm the change will not bring any potential issue. After that, we can try to apply it to a wider range. And I really don't want to update all the instances of Param[String] before we agree what's the best practice for some implementation details. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #19599: [SPARK-22381] [ML] Add StringParam that supports valid o...
Github user attilapiros commented on the issue: https://github.com/apache/spark/pull/19599 As I see inside the StringParam the options even can be mapped to enum values. This way ignoring cases at the matching part would be no problem at all (and it would be very efficient). Going further this could be a type safe construct by making StringParam a generic where the generic parameter is the enum type used as the target of the mapping: like StringParam[NaiveBayesModelType]. What is your opinion? --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #19599: [SPARK-22381] [ML] Add StringParam that supports valid o...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/19599 Merged build finished. Test FAILed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #19599: [SPARK-22381] [ML] Add StringParam that supports valid o...
Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/19599 **[Test build #84699 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84699/testReport)** for PR 19599 at commit [`f55b22d`](https://github.com/apache/spark/commit/f55b22dacef04b3d3e6f0cb05acad3e9997c133d). * This patch **fails to generate documentation**. * This patch merges cleanly. * This patch adds no public classes. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #19599: [SPARK-22381] [ML] Add StringParam that supports valid o...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/19599 Test FAILed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/84699/ Test FAILed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #19599: [SPARK-22381] [ML] Add StringParam that supports valid o...
Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/19599 **[Test build #84699 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84699/testReport)** for PR 19599 at commit [`f55b22d`](https://github.com/apache/spark/commit/f55b22dacef04b3d3e6f0cb05acad3e9997c133d). --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #19599: [SPARK-22381] [ML] Add StringParam that supports valid o...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/19599 Test FAILed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/84698/ Test FAILed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #19599: [SPARK-22381] [ML] Add StringParam that supports valid o...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/19599 Merged build finished. Test FAILed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #19599: [SPARK-22381] [ML] Add StringParam that supports valid o...
Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/19599 **[Test build #84698 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84698/testReport)** for PR 19599 at commit [`3f6e472`](https://github.com/apache/spark/commit/3f6e472ebff8327e20dcdb0041f525a3856244b0). * This patch **fails MiMa tests**. * This patch merges cleanly. * This patch adds the following public classes _(experimental)_: * `class StringParam(parent: Params, name: String, doc: String, isValid: String => Boolean)` --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #19599: [SPARK-22381] [ML] Add StringParam that supports valid o...
Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/19599 **[Test build #84698 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84698/testReport)** for PR 19599 at commit [`3f6e472`](https://github.com/apache/spark/commit/3f6e472ebff8327e20dcdb0041f525a3856244b0). --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #19599: [SPARK-22381] [ML] Add StringParam that supports valid o...
Github user hhbyyh commented on the issue: https://github.com/apache/spark/pull/19599 I used two ways to switch String params among different options: 1. In NaiveBayes: convert StringParam and String constants to lowercase. 2. in LinearRegression: .equalsIgnoreCase Currently most Spark algorithms use 1. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #19599: [SPARK-22381] [ML] Add StringParam that supports valid o...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/19599 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/83199/ Test PASSed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #19599: [SPARK-22381] [ML] Add StringParam that supports valid o...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/19599 Merged build finished. Test PASSed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #19599: [SPARK-22381] [ML] Add StringParam that supports valid o...
Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/19599 **[Test build #83199 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/83199/testReport)** for PR 19599 at commit [`01e7d3d`](https://github.com/apache/spark/commit/01e7d3d5f9b0ae278ebce60635e5c2568d3d0cf3). * This patch passes all tests. * This patch merges cleanly. * This patch adds no public classes. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #19599: [SPARK-22381] [ML] Add StringParam that supports valid o...
Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/19599 **[Test build #83199 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/83199/testReport)** for PR 19599 at commit [`01e7d3d`](https://github.com/apache/spark/commit/01e7d3d5f9b0ae278ebce60635e5c2568d3d0cf3). --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #19599: [SPARK-22381] [ML] Add StringParam that supports valid o...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/19599 Test FAILed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/83174/ Test FAILed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #19599: [SPARK-22381] [ML] Add StringParam that supports valid o...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/19599 Merged build finished. Test FAILed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #19599: [SPARK-22381] [ML] Add StringParam that supports valid o...
Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/19599 **[Test build #83174 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/83174/testReport)** for PR 19599 at commit [`f6b918e`](https://github.com/apache/spark/commit/f6b918eda3448df054edd81a077fa1a3d996d5c1). * This patch **fails MiMa tests**. * This patch merges cleanly. * This patch adds no public classes. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #19599: [SPARK-22381] [ML] Add StringParam that supports valid o...
Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/19599 **[Test build #83174 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/83174/testReport)** for PR 19599 at commit [`f6b918e`](https://github.com/apache/spark/commit/f6b918eda3448df054edd81a077fa1a3d996d5c1). --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org