You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by sethah <gi...@git.apache.org> on 2016/01/28 01:06:09 UTC
[GitHub] spark pull request: [SPARK-13047][PYSPARK][ML] Pyspark Params.hasP...
GitHub user sethah opened a pull request:
https://github.com/apache/spark/pull/10962
[SPARK-13047][PYSPARK][ML] Pyspark Params.hasParam should not throw an error
Pyspark Params class has a method `hasParam(paramName)` which returns `True` if the class has a parameter by that name, but throws an `AttributeError` otherwise. There is not currently a way of getting a Boolean to indicate if a class has a parameter. With Spark 2.0 we could modify the existing behavior of `hasParam` or add an additional method with this functionality.
In Python:
```python
from pyspark.ml.classification import NaiveBayes
nb = NaiveBayes(smoothing=0.5)
print nb.hasParam("smoothing")
print nb.hasParam("notAParam")
```
produces:
> True
> AttributeError: 'NaiveBayes' object has no attribute 'notAParam'
However, in Scala:
```scala
import org.apache.spark.ml.classification.NaiveBayes
val nb = new NaiveBayes()
nb.hasParam("smoothing")
nb.hasParam("notAParam")
```
produces:
> true
> false
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/sethah/spark SPARK-13047
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/10962.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 #10962
----
commit d52b1de1adefedb6938130d0530ea46fdb3f64f7
Author: sethah <se...@gmail.com>
Date: 2016-01-27T23:55:04Z
hasParam returns False instead of throwing an error
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13047][PYSPARK][ML] Pyspark Params.hasP...
Posted by sethah <gi...@git.apache.org>.
Github user sethah commented on the pull request:
https://github.com/apache/spark/pull/10962#issuecomment-180485241
@davies Thanks for taking a look! I have addressed your comment.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13047][PYSPARK][ML] Pyspark Params.hasP...
Posted by mengxr <gi...@git.apache.org>.
Github user mengxr commented on the pull request:
https://github.com/apache/spark/pull/10962#issuecomment-183128117
Merged into master and branch-1.6. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13047][PYSPARK][ML] Pyspark Params.hasP...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10962#issuecomment-175920740
**[Test build #50242 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/50242/consoleFull)** for PR 10962 at commit [`d52b1de`](https://github.com/apache/spark/commit/d52b1de1adefedb6938130d0530ea46fdb3f64f7).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13047][PYSPARK][ML] Pyspark Params.hasP...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10962#issuecomment-183124943
**[Test build #51152 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/51152/consoleFull)** for PR 10962 at commit [`6581575`](https://github.com/apache/spark/commit/6581575becf01c5f04530bf8c8723c77d9d6bae1).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13047][PYSPARK][ML] Pyspark Params.hasP...
Posted by holdenk <gi...@git.apache.org>.
Github user holdenk commented on the pull request:
https://github.com/apache/spark/pull/10962#issuecomment-176323543
Looking at params.scala shouldOwn uses a require which we should probably stay consistent with (not saying we _shouldn't_ change it per-se just if we do we should change both). I think throwing an exception is more reasonable for shouldOwn since its only an internal use but hasParam is external facing.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13047][PYSPARK][ML] Pyspark Params.hasP...
Posted by mengxr <gi...@git.apache.org>.
Github user mengxr commented on the pull request:
https://github.com/apache/spark/pull/10962#issuecomment-183109271
@sethah The changes look good to me and thanks for fixing it! I would say this is a bug and we should backport it to branch-1.6. So the note on the behavior change is unnecessary.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13047][PYSPARK][ML] Pyspark Params.hasP...
Posted by yanboliang <gi...@git.apache.org>.
Github user yanboliang commented on a diff in the pull request:
https://github.com/apache/spark/pull/10962#discussion_r51087340
--- Diff: python/pyspark/ml/param/__init__.py ---
@@ -152,13 +152,17 @@ def isDefined(self, param):
return self.isSet(param) or self.hasDefault(param)
@since("1.4.0")
- def hasParam(self, paramName):
+ def hasParam(self, param):
"""
- Tests whether this instance contains a param with a given
- (string) name.
+ Tests whether this instance contains a param.
"""
- param = self._resolveParam(paramName)
- return param in self.params
+ if isinstance(param, Param):
+ return hasattr(self, param.name)
--- End diff --
+1 for accepting only strings. If no strong reasons, keep consistent with Scala is the best choice.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13047][PYSPARK][ML] Pyspark Params.hasP...
Posted by sethah <gi...@git.apache.org>.
Github user sethah commented on the pull request:
https://github.com/apache/spark/pull/10962#issuecomment-183112906
Thanks for reviewing @mengxr! I have addressed your comment.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13047][PYSPARK][ML] Pyspark Params.hasP...
Posted by sethah <gi...@git.apache.org>.
Github user sethah commented on a diff in the pull request:
https://github.com/apache/spark/pull/10962#discussion_r51188359
--- Diff: python/pyspark/ml/param/__init__.py ---
@@ -156,9 +156,16 @@ def hasParam(self, paramName):
"""
Tests whether this instance contains a param with a given
(string) name.
+
+ Note: this method is changed in Spark 2.0+ to return `False`
--- End diff --
Left a note per @holdenk 's suggestion. I can reword if needed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13047][PYSPARK][ML] Pyspark Params.hasP...
Posted by sethah <gi...@git.apache.org>.
Github user sethah commented on the pull request:
https://github.com/apache/spark/pull/10962#issuecomment-180478448
ping! @yanboliang @jkbradley
This is a small change that will allow the Python API to match the Scala API, where the current behavior could be reasonably stated as a bug. Is there anything I need to change or update on the PR? I would very much appreciate your review.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13047][PYSPARK][ML] Pyspark Params.hasP...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/10962
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13047][PYSPARK][ML] Pyspark Params.hasP...
Posted by holdenk <gi...@git.apache.org>.
Github user holdenk commented on the pull request:
https://github.com/apache/spark/pull/10962#issuecomment-176439329
Maybe @davies or @jkbradley could take a look?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13047][PYSPARK][ML] Pyspark Params.hasP...
Posted by davies <gi...@git.apache.org>.
Github user davies commented on the pull request:
https://github.com/apache/spark/pull/10962#issuecomment-180480810
LGTM, just one minor comment.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13047][PYSPARK][ML] Pyspark Params.hasP...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10962#issuecomment-183118493
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/51147/
Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13047][PYSPARK][ML] Pyspark Params.hasP...
Posted by sethah <gi...@git.apache.org>.
Github user sethah commented on a diff in the pull request:
https://github.com/apache/spark/pull/10962#discussion_r51076979
--- Diff: python/pyspark/ml/param/__init__.py ---
@@ -152,13 +152,17 @@ def isDefined(self, param):
return self.isSet(param) or self.hasDefault(param)
@since("1.4.0")
- def hasParam(self, paramName):
+ def hasParam(self, param):
"""
- Tests whether this instance contains a param with a given
- (string) name.
+ Tests whether this instance contains a param.
"""
- param = self._resolveParam(paramName)
- return param in self.params
+ if isinstance(param, Param):
+ return hasattr(self, param.name)
--- End diff --
I tend to agree that we should only accept string types for this function. The reason I have included `Param` type is because in the current ml param tests, there is a check where `hasParam` is called by passing a `Param` instance instead of a string, so this test would fail ([see here](https://github.com/apache/spark/blob/master/python/pyspark/ml/tests.py#L216)). It is odd that the test passes a `Param` instance and not a string, since the function describes itself as accepting strings, but, in an odd twist, the check works anyway.
If we do accept `Param` type, we can't call `_shouldOwn` because it throws an error instead of returning `False` (by design?). At any rate, I vote to accept only strings and change the test to pass in the param name instead of the param.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13047][PYSPARK][ML] Pyspark Params.hasP...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10962#issuecomment-176278459
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/50277/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13047][PYSPARK][ML] Pyspark Params.hasP...
Posted by sethah <gi...@git.apache.org>.
Github user sethah commented on the pull request:
https://github.com/apache/spark/pull/10962#issuecomment-176319087
I wonder if there is support for also changing `_shouldOwn` to return True/False instead of True/ValueError? The only placed it is currently used is in `_resolveParam`, which could be modified to keep the same behavior. There is a related PR that would benefit from a `_shouldOwn` True/False check. Plus, the name seems to imply returning True/False instead of throwing an error. Thoughts?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13047][PYSPARK][ML] Pyspark Params.hasP...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10962#issuecomment-176432322
**[Test build #50296 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/50296/consoleFull)** for PR 10962 at commit [`a1b885c`](https://github.com/apache/spark/commit/a1b885c5406d339a4e016a61a01defc3e7897310).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13047][PYSPARK][ML] Pyspark Params.hasP...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10962#issuecomment-176278457
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13047][PYSPARK][ML] Pyspark Params.hasP...
Posted by mengxr <gi...@git.apache.org>.
Github user mengxr commented on the pull request:
https://github.com/apache/spark/pull/10962#issuecomment-183118677
test this please
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13047][PYSPARK][ML] Pyspark Params.hasP...
Posted by sethah <gi...@git.apache.org>.
Github user sethah commented on the pull request:
https://github.com/apache/spark/pull/10962#issuecomment-176325696
Good point about mirroring Scala. In that case, it is probably best not to change it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13047][PYSPARK][ML] Pyspark Params.hasP...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10962#issuecomment-180499618
**[Test build #50826 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/50826/consoleFull)** for PR 10962 at commit [`8dc7d05`](https://github.com/apache/spark/commit/8dc7d05a15cc6273e69f1a3803194bd9a3883348).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13047][PYSPARK][ML] Pyspark Params.hasP...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10962#issuecomment-176426837
**[Test build #50296 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/50296/consoleFull)** for PR 10962 at commit [`a1b885c`](https://github.com/apache/spark/commit/a1b885c5406d339a4e016a61a01defc3e7897310).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13047][PYSPARK][ML] Pyspark Params.hasP...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10962#issuecomment-176432631
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/50296/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13047][PYSPARK][ML] Pyspark Params.hasP...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10962#issuecomment-183125211
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/51152/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13047][PYSPARK][ML] Pyspark Params.hasP...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10962#issuecomment-176269418
**[Test build #50277 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/50277/consoleFull)** for PR 10962 at commit [`2989f93`](https://github.com/apache/spark/commit/2989f9335613d94d5b08909e316ceee449cc94d7).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13047][PYSPARK][ML] Pyspark Params.hasP...
Posted by davies <gi...@git.apache.org>.
Github user davies commented on a diff in the pull request:
https://github.com/apache/spark/pull/10962#discussion_r52052341
--- Diff: python/pyspark/ml/param/__init__.py ---
@@ -156,9 +156,16 @@ def hasParam(self, paramName):
"""
Tests whether this instance contains a param with a given
(string) name.
+
+ Note: this method is changed in Spark 2.0+ to return `False`
+ instead of raising `AttributeError` when the instance
+ does not have a parameter with `paramName`.
"""
- param = self._resolveParam(paramName)
- return param in self.params
+ if isinstance(paramName, str):
+ p = getattr(self, paramName, None)
+ return p is not None and isinstance(p, Param)
--- End diff --
nit: `p is not None` is not needed
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13047][PYSPARK][ML] Pyspark Params.hasP...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10962#issuecomment-180499905
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13047][PYSPARK][ML] Pyspark Params.hasP...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10962#issuecomment-176432626
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13047][PYSPARK][ML] Pyspark Params.hasP...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10962#issuecomment-180499795
**[Test build #2521 has finished](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/2521/consoleFull)** for PR 10962 at commit [`8dc7d05`](https://github.com/apache/spark/commit/8dc7d05a15cc6273e69f1a3803194bd9a3883348).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13047][PYSPARK][ML] Pyspark Params.hasP...
Posted by yanboliang <gi...@git.apache.org>.
Github user yanboliang commented on a diff in the pull request:
https://github.com/apache/spark/pull/10962#discussion_r51075753
--- Diff: python/pyspark/ml/param/__init__.py ---
@@ -152,13 +152,17 @@ def isDefined(self, param):
return self.isSet(param) or self.hasDefault(param)
@since("1.4.0")
- def hasParam(self, paramName):
+ def hasParam(self, param):
"""
- Tests whether this instance contains a param with a given
- (string) name.
+ Tests whether this instance contains a param.
"""
- param = self._resolveParam(paramName)
- return param in self.params
+ if isinstance(param, Param):
+ return hasattr(self, param.name)
--- End diff --
If we support ```param``` of type ```Param```, we should not only check the ```hasattr(self, param.name)``` but also check ```self.uid == param.parent```. You can directly call ```_shouldOwn``` to do this work. It means if you provide a ```Param``` to check whether it belongs to the instance, you should both check ```uid``` and ```name```.
I vote to only support ```paramName``` that make consistent semantics with Scala.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13047][PYSPARK][ML] Pyspark Params.hasP...
Posted by mengxr <gi...@git.apache.org>.
Github user mengxr commented on a diff in the pull request:
https://github.com/apache/spark/pull/10962#discussion_r52686031
--- Diff: python/pyspark/ml/param/__init__.py ---
@@ -156,9 +156,16 @@ def hasParam(self, paramName):
"""
Tests whether this instance contains a param with a given
(string) name.
+
+ Note: this method is changed in Spark 2.0+ to return `False`
--- End diff --
I would view this one as a bug fix and backport it to branch-1.6. So the note is not necessary.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13047][PYSPARK][ML] Pyspark Params.hasP...
Posted by holdenk <gi...@git.apache.org>.
Github user holdenk commented on the pull request:
https://github.com/apache/spark/pull/10962#issuecomment-175921366
I think this makes sense since it is harmonizing the behaviour with the Scala API. Perhaps adding a note in the PyDoc about the new behaviour as well as a test might be good additions to the PR?
Also lets ping @jkbradley & @yanboliang for thoughts on changing the API like this.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13047][PYSPARK][ML] Pyspark Params.hasP...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10962#issuecomment-176278153
**[Test build #50277 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/50277/consoleFull)** for PR 10962 at commit [`2989f93`](https://github.com/apache/spark/commit/2989f9335613d94d5b08909e316ceee449cc94d7).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13047][PYSPARK][ML] Pyspark Params.hasP...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10962#issuecomment-180494282
**[Test build #50826 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/50826/consoleFull)** for PR 10962 at commit [`8dc7d05`](https://github.com/apache/spark/commit/8dc7d05a15cc6273e69f1a3803194bd9a3883348).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13047][PYSPARK][ML] Pyspark Params.hasP...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10962#issuecomment-183125205
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13047][PYSPARK][ML] Pyspark Params.hasP...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10962#issuecomment-183118490
Merged build finished. Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13047][PYSPARK][ML] Pyspark Params.hasP...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10962#issuecomment-180494296
**[Test build #2521 has started](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/2521/consoleFull)** for PR 10962 at commit [`8dc7d05`](https://github.com/apache/spark/commit/8dc7d05a15cc6273e69f1a3803194bd9a3883348).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13047][PYSPARK][ML] Pyspark Params.hasP...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10962#issuecomment-180499908
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/50826/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13047][PYSPARK][ML] Pyspark Params.hasP...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10962#issuecomment-183120615
**[Test build #51152 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/51152/consoleFull)** for PR 10962 at commit [`6581575`](https://github.com/apache/spark/commit/6581575becf01c5f04530bf8c8723c77d9d6bae1).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org