Skip to content

[SPARK-15741][PYSPARK][ML] Pyspark cleanup of set default seed to None#13672

Closed
BryanCutler wants to merge 3 commits intoapache:masterfrom
BryanCutler:pyspark-cleanup-setDefault-seed-SPARK-15741
Closed

[SPARK-15741][PYSPARK][ML] Pyspark cleanup of set default seed to None#13672
BryanCutler wants to merge 3 commits intoapache:masterfrom
BryanCutler:pyspark-cleanup-setDefault-seed-SPARK-15741

Conversation

@BryanCutler
Copy link
Member

@BryanCutler BryanCutler commented Jun 14, 2016

What changes were proposed in this pull request?

Several places set the seed Param default value to None which will translate to a zero value on the Scala side. This is unnecessary because a default fixed value already exists and if a test depends on a zero valued seed, then it should explicitly set it to zero instead of relying on this translation. These cases can be safely removed except for the ALS doc test, which has been changed to set the seed value to zero.

How was this patch tested?

Ran PySpark tests locally

@BryanCutler
Copy link
Member Author

@MLnick , does this seem reasonable?
cc @yanboliang

@SparkQA
Copy link

SparkQA commented Jun 14, 2016

Test build #60520 has finished for PR 13672 at commit abe997d.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@MLnick
Copy link
Contributor

MLnick commented Jun 17, 2016

LGTM

@BryanCutler
Copy link
Member Author

Thanks @MLnick , can this be merged or do we need to wait until 2.0 is cut?

@MLnick
Copy link
Contributor

MLnick commented Jun 21, 2016

jenkins retest this please

@SparkQA
Copy link

SparkQA commented Jun 21, 2016

Test build #60911 has finished for PR 13672 at commit abe997d.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@mengxr
Copy link
Contributor

mengxr commented Jun 21, 2016

Merged into master and branch-2.0. Thanks!

asfgit pushed a commit that referenced this pull request Jun 21, 2016
## What changes were proposed in this pull request?

Several places set the seed Param default value to None which will translate to a zero value on the Scala side.  This is unnecessary because a default fixed value already exists and if a test depends on a zero valued seed, then it should explicitly set it to zero instead of relying on this translation.  These cases can be safely removed except for the ALS doc test, which has been changed to set the seed value to zero.

## How was this patch tested?

Ran PySpark tests locally

Author: Bryan Cutler <cutlerb@gmail.com>

Closes #13672 from BryanCutler/pyspark-cleanup-setDefault-seed-SPARK-15741.

(cherry picked from commit b76e355)
Signed-off-by: Xiangrui Meng <meng@databricks.com>
@asfgit asfgit closed this in b76e355 Jun 21, 2016
@BryanCutler
Copy link
Member Author

Thanks @mengxr and @MLnick!

@BryanCutler BryanCutler deleted the pyspark-cleanup-setDefault-seed-SPARK-15741 branch December 2, 2016 00:59
@BryanCutler BryanCutler restored the pyspark-cleanup-setDefault-seed-SPARK-15741 branch December 2, 2016 01:00
@BryanCutler BryanCutler deleted the pyspark-cleanup-setDefault-seed-SPARK-15741 branch January 3, 2017 22:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants