[GitHub] spark issue #19599: [SPARK-22381] [ML] Add StringParam that supports valid o...

2018-03-20 Thread hhbyyh
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...

2017-12-13 Thread AmplabJenkins
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...

2017-12-13 Thread AmplabJenkins
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...

2017-12-13 Thread SparkQA
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...

2017-12-13 Thread hhbyyh
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...

2017-12-13 Thread SparkQA
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...

2017-12-13 Thread hhbyyh
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...

2017-12-12 Thread hhbyyh
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...

2017-12-12 Thread attilapiros
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...

2017-12-10 Thread AmplabJenkins
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...

2017-12-10 Thread SparkQA
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...

2017-12-10 Thread AmplabJenkins
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...

2017-12-10 Thread SparkQA
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...

2017-12-10 Thread AmplabJenkins
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...

2017-12-10 Thread AmplabJenkins
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...

2017-12-10 Thread SparkQA
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...

2017-12-10 Thread SparkQA
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...

2017-11-01 Thread hhbyyh
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...

2017-10-29 Thread AmplabJenkins
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...

2017-10-29 Thread AmplabJenkins
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...

2017-10-29 Thread SparkQA
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...

2017-10-29 Thread SparkQA
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...

2017-10-28 Thread AmplabJenkins
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...

2017-10-28 Thread AmplabJenkins
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...

2017-10-28 Thread SparkQA
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...

2017-10-28 Thread SparkQA
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