You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by hujy <gi...@git.apache.org> on 2016/05/04 06:16:54 UTC
[GitHub] spark pull request: [SPARK-14772][ML,PySpark]Python ML Params.copy...
GitHub user hujy opened a pull request:
https://github.com/apache/spark/pull/12888
[SPARK-14772][ML,PySpark]Python ML Params.copy treats uid, paramMaps …
[SPARK-14772](https://issues.apache.org/jira/browse/SPARK-14772#)
##What changes were proposed in this pull request?
In PySpark, ml.param.Params.copy does not quite match the Scala implementation:
(1) It does not copy the UID.
(2) It does not respect the difference between defaultParamMap and paramMap. This is an issue with _copyValues.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/hujy/spark 14772
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/12888.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 #12888
----
commit 2a05414b5c8dcec1a2d3c2389ecbc1cdd498b6bb
Author: HuJiayin <ji...@intel.com>
Date: 2016-05-04T06:08:12Z
[SPARK-14772][ML,PySpark]Python ML Params.copy treats uid, paramMaps differently than 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-14772][ML,PySpark]Python ML Params.copy...
Posted by holdenk <gi...@git.apache.org>.
Github user holdenk commented on a diff in the pull request:
https://github.com/apache/spark/pull/12888#discussion_r61994856
--- Diff: python/pyspark/ml/param/__init__.py ---
@@ -357,7 +357,7 @@ def getOrDefault(self, param):
return self._defaultParamMap[param]
@since("1.4.0")
- def extractParamMap(self, extra=None):
--- End diff --
Please document this new 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-14772][ML,PySpark]Python ML Params.copy...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12888#issuecomment-221022337
Can one of the admins verify this patch?
---
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-14772][ML,PySpark]Python ML Params.copy...
Posted by holdenk <gi...@git.apache.org>.
Github user holdenk commented on the pull request:
https://github.com/apache/spark/pull/12888#issuecomment-216761853
Also it would be good to have tests to ensure the change has the desired impact.
---
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-14772][ML,PySpark]Python ML Params.copy...
Posted by hujy <gi...@git.apache.org>.
Github user hujy commented on the pull request:
https://github.com/apache/spark/pull/12888#issuecomment-217081026
@holdenk, I'm checking with @jkbradley about "respect the difference between defaultParamMap and paramMap" in the requirement. The new input parameter is optional and will not impact the user not want to copy default 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-14772][ML,PySpark]Python ML Params.copy...
Posted by hujy <gi...@git.apache.org>.
Github user hujy commented on the pull request:
https://github.com/apache/spark/pull/12888#issuecomment-218922641
ok with test
---
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 issue #12888: [SPARK-14772][ML,PySpark]Python ML Params.copy treats ui...
Posted by jkbradley <gi...@git.apache.org>.
Github user jkbradley commented on the issue:
https://github.com/apache/spark/pull/12888
@hujy Thank you for sending this PR, and apologies for not seeing it earlier. Since the other PR for this JIRA is ready to merge, could you please close this issue? Thanks again!
---
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-14772][ML,PySpark]Python ML Params.copy...
Posted by holdenk <gi...@git.apache.org>.
Github user holdenk commented on the pull request:
https://github.com/apache/spark/pull/12888#issuecomment-216759643
Thanks for tackling this issue :) For a better understanding - is there a reason why adding a flag for this behaviour instead of just changing it (since it is a bug) - do we expect people to want to explicitly copy the default param map in this way?
---
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 #12888: [SPARK-14772][ML,PySpark]Python ML Params.copy tr...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/12888
---
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-14772][ML,PySpark]Python ML Params.copy...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12888#issuecomment-216755365
Can one of the admins verify this patch?
---
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 issue #12888: [SPARK-14772][ML,PySpark]Python ML Params.copy treats ui...
Posted by holdenk <gi...@git.apache.org>.
Github user holdenk commented on the issue:
https://github.com/apache/spark/pull/12888
So to trigger the test we will need one of the committers - e.g. @davies is one of the more active Python committers (although he has been busy lately) so we can also check with @MLnick . It would also be nice if you could document the param (even if we don't expect the user to use it - its useful to have it noted for other developers and so users know not to worry about 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-14772][ML,PySpark]Python ML Params.copy...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12888#issuecomment-221454090
Can one of the admins verify this patch?
---
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