You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by mpjlu <gi...@git.apache.org> on 2017/05/23 06:29:49 UTC
[GitHub] spark pull request #18068: [SPARK-20764][ML][PySpark][FOLLOWUP]Fix visibilit...
GitHub user mpjlu opened a pull request:
https://github.com/apache/spark/pull/18068
[SPARK-20764][ML][PySpark][FOLLOWUP]Fix visibility discrepancy with numInstances and degreesOfFreedom in LR and GLR - Python version
## What changes were proposed in this pull request?
Add test cases for PR-18062
## How was this patch tested?
The existing UT
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/mpjlu/spark moreTest
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/18068.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 #18068
----
commit 6b31ec7dda73c155fc94c5ccf53709099f8033dd
Author: Peng <pe...@intel.com>
Date: 2017-05-22T11:37:50Z
fix visibility of numInstances and degreesOfFreedom in LR and GLR - Python version
commit a8b407f877269f235611e5dc5bb338c421206a57
Author: Peng <pe...@intel.com>
Date: 2017-05-23T05:52:29Z
follow up of SPARK-20764
----
---
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 #18068: [SPARK-20764][ML][PySpark][FOLLOWUP]Fix visibilit...
Posted by mpjlu <gi...@git.apache.org>.
Github user mpjlu commented on a diff in the pull request:
https://github.com/apache/spark/pull/18068#discussion_r117915858
--- Diff: python/pyspark/ml/tests.py ---
@@ -1075,7 +1076,8 @@ def test_linear_regression_summary(self):
pValues = s.pValues
self.assertTrue(isinstance(pValues, list) and isinstance(pValues[0], float))
# test evaluation (with training dataset) produces a summary with same values
- # one check is enough to verify a summary is returned, Scala version runs full test
+ # one check is enough to verify a summary is returned
+ # The child class LinearRegressionTrainingSummary runs full test
--- End diff --
The comment is not related with this PR, just because the previous comment is misleading.
---
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 #18068: [SPARK-20764][ML][PySpark][FOLLOWUP]Fix visibilit...
Posted by MLnick <gi...@git.apache.org>.
Github user MLnick commented on a diff in the pull request:
https://github.com/apache/spark/pull/18068#discussion_r117912043
--- Diff: python/pyspark/ml/tests.py ---
@@ -1075,7 +1076,8 @@ def test_linear_regression_summary(self):
pValues = s.pValues
self.assertTrue(isinstance(pValues, list) and isinstance(pValues[0], float))
# test evaluation (with training dataset) produces a summary with same values
- # one check is enough to verify a summary is returned, Scala version runs full test
+ # one check is enough to verify a summary is returned
+ # The child class LinearRegressionTrainingSummary runs full test
--- End diff --
I'm not sure what this comment means?
---
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 #18068: [SPARK-20764][ML][PySpark][FOLLOWUP]Fix visibility discr...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18068
**[Test build #77229 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/77229/testReport)** for PR 18068 at commit [`7bbfe3a`](https://github.com/apache/spark/commit/7bbfe3a860964d166f67c3b099b00c8b11a73f9d).
---
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 #18068: [SPARK-20764][ML][PySpark][FOLLOWUP]Fix visibility discr...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18068
**[Test build #77234 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/77234/testReport)** for PR 18068 at commit [`013adc4`](https://github.com/apache/spark/commit/013adc4460c588e1e06a66d23ce66d864803554e).
* 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 issue #18068: [SPARK-20764][ML][PySpark][FOLLOWUP]Fix visibility discr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18068
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 #18068: [SPARK-20764][ML][PySpark][FOLLOWUP]Fix visibilit...
Posted by mpjlu <gi...@git.apache.org>.
GitHub user mpjlu reopened a pull request:
https://github.com/apache/spark/pull/18068
[SPARK-20764][ML][PySpark][FOLLOWUP]Fix visibility discrepancy with numInstances and degreesOfFreedom in LR and GLR - Python version
## What changes were proposed in this pull request?
Add test cases for PR-18062
## How was this patch tested?
The existing UT
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/mpjlu/spark moreTest
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/18068.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 #18068
----
commit 6b31ec7dda73c155fc94c5ccf53709099f8033dd
Author: Peng <pe...@intel.com>
Date: 2017-05-22T11:37:50Z
fix visibility of numInstances and degreesOfFreedom in LR and GLR - Python version
commit a8b407f877269f235611e5dc5bb338c421206a57
Author: Peng <pe...@intel.com>
Date: 2017-05-23T05:52:29Z
follow up of SPARK-20764
commit 7bbfe3a860964d166f67c3b099b00c8b11a73f9d
Author: Peng <pe...@intel.com>
Date: 2017-05-23T05:58:52Z
Merge remote-tracking branch 'origin/master' into moreTest
----
---
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 #18068: [SPARK-20764][ML][PySpark][FOLLOWUP]Fix visibilit...
Posted by mpjlu <gi...@git.apache.org>.
Github user mpjlu closed the pull request at:
https://github.com/apache/spark/pull/18068
---
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 #18068: [SPARK-20764][ML][PySpark][FOLLOWUP]Fix visibility discr...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18068
**[Test build #77234 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/77234/testReport)** for PR 18068 at commit [`013adc4`](https://github.com/apache/spark/commit/013adc4460c588e1e06a66d23ce66d864803554e).
---
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 #18068: [SPARK-20764][ML][PySpark][FOLLOWUP]Fix visibility discr...
Posted by mpjlu <gi...@git.apache.org>.
Github user mpjlu commented on the issue:
https://github.com/apache/spark/pull/18068
retest 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 issue #18068: [SPARK-20764][ML][PySpark][FOLLOWUP]Fix visibility discr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18068
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 #18068: [SPARK-20764][ML][PySpark][FOLLOWUP]Fix visibilit...
Posted by yanboliang <gi...@git.apache.org>.
Github user yanboliang commented on a diff in the pull request:
https://github.com/apache/spark/pull/18068#discussion_r118231105
--- Diff: python/pyspark/ml/tests.py ---
@@ -1075,7 +1076,8 @@ def test_linear_regression_summary(self):
pValues = s.pValues
self.assertTrue(isinstance(pValues, list) and isinstance(pValues[0], float))
# test evaluation (with training dataset) produces a summary with same values
- # one check is enough to verify a summary is returned, Scala version runs full test
+ # one check is enough to verify a summary is returned
+ # The child class LinearRegressionTrainingSummary runs full test
--- End diff --
@MLnick This is because the type of ```LinearRegressionModel.summary``` is ```LinearRegressionTrainingSummary```, but the return type of ```LinearRegressionModel.evalute()``` is ```LinearRegressionSummary```. Theoretically we should test both, but we can simplify the test for only check one function since we have checked all functions in the child class. Thanks for @mpjlu to update this comments.
---
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 #18068: [SPARK-20764][ML][PySpark][FOLLOWUP]Fix visibility discr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18068
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/77229/
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 issue #18068: [SPARK-20764][ML][PySpark][FOLLOWUP]Fix visibility discr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18068
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/77234/
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 #18068: [SPARK-20764][ML][PySpark][FOLLOWUP]Fix visibilit...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/18068
---
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 #18068: [SPARK-20764][ML][PySpark][FOLLOWUP]Fix visibility discr...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18068
**[Test build #77231 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/77231/testReport)** for PR 18068 at commit [`013adc4`](https://github.com/apache/spark/commit/013adc4460c588e1e06a66d23ce66d864803554e).
---
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 #18068: [SPARK-20764][ML][PySpark][FOLLOWUP]Fix visibility discr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18068
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 issue #18068: [SPARK-20764][ML][PySpark][FOLLOWUP]Fix visibility discr...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18068
**[Test build #77229 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/77229/testReport)** for PR 18068 at commit [`7bbfe3a`](https://github.com/apache/spark/commit/7bbfe3a860964d166f67c3b099b00c8b11a73f9d).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* ` class DecisionTreeClassifierWrapperWriter(instance: DecisionTreeClassifierWrapper)`
* ` class DecisionTreeClassifierWrapperReader extends MLReader[DecisionTreeClassifierWrapper] `
* ` class DecisionTreeRegressorWrapperWriter(instance: DecisionTreeRegressorWrapper)`
* ` class DecisionTreeRegressorWrapperReader extends MLReader[DecisionTreeRegressorWrapper] `
---
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 #18068: [SPARK-20764][ML][PySpark][FOLLOWUP]Fix visibilit...
Posted by mpjlu <gi...@git.apache.org>.
Github user mpjlu commented on a diff in the pull request:
https://github.com/apache/spark/pull/18068#discussion_r117912054
--- Diff: python/pyspark/ml/tests.py ---
@@ -1075,7 +1076,8 @@ def test_linear_regression_summary(self):
pValues = s.pValues
self.assertTrue(isinstance(pValues, list) and isinstance(pValues[0], float))
# test evaluation (with training dataset) produces a summary with same values
- # one check is enough to verify a summary is returned, Scala version runs full test
+ # one check is enough to verify a summary is returned
+ # The child class LinearRegressionTrainingSummary runs full test
--- End diff --
I think this is not because Scala version runs full test. Even Scala version runs full test, we still need the function call test.
If a child class have done the function call test, we don't need to test parent class 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 issue #18068: [SPARK-20764][ML][PySpark][FOLLOWUP]Fix visibility discr...
Posted by yanboliang <gi...@git.apache.org>.
Github user yanboliang commented on the issue:
https://github.com/apache/spark/pull/18068
LGTM, merged into master and branch-2.2. Thanks for all.
---
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 #18068: [SPARK-20764][ML][PySpark][FOLLOWUP]Fix visibility discr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18068
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/77231/
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