Skip to content

Conversation

@jkbradley
Copy link
Member

Extend CrossValidator with HasSeed in PySpark.

This PR replaces [https://github.com//pull/7997]

CC: @yanboliang @thunterdb @mmenestret Would one of you mind taking a look? Thanks!

@SparkQA
Copy link

SparkQA commented Dec 11, 2015

Test build #47591 has finished for PR 10268 at commit 85aae68.

  • This patch fails PySpark unit tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):\n * class CrossValidator(Estimator, HasSeed):\n

@SparkQA
Copy link

SparkQA commented Dec 11, 2015

Test build #2210 has finished for PR 10268 at commit 2f60607.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):\n * class CrossValidator(Estimator, HasSeed):\n * class ExecutorClassLoader(\n

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add \ at the end of line, otherwise it can not generate python doc correctly.

@yanboliang
Copy link
Contributor

LGTM except minor issues.

@jkbradley
Copy link
Member Author

@yanboliang Thanks! With the fix, it worked when I generated the docs locally.

@SparkQA
Copy link

SparkQA commented Dec 16, 2015

Test build #47766 has finished for PR 10268 at commit 0bd1457.

  • This patch fails RAT tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):\n * class CrossValidator(Estimator, HasSeed):\n

@SparkQA
Copy link

SparkQA commented Dec 16, 2015

Test build #2219 has finished for PR 10268 at commit 0bd1457.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):\n * class CrossValidator(Estimator, HasSeed):\n

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We set default value of seed to this.getClass.getName.hashCode.toLong at Scala side. I think we should keep consistent with that, otherwise users may get different result when training with the same dataset.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just an example though. But I agree we could remove the seed now to simplify the example. I'll do that.

@yanboliang
Copy link
Contributor

@jkbradley I think it's the last issue that we should resolve, thanks!

@SparkQA
Copy link

SparkQA commented Dec 16, 2015

Test build #47844 has finished for PR 10268 at commit 9bf75ad.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):\n * class CrossValidator(Estimator, HasSeed):\n

@asfgit asfgit closed this in 3a44aeb Dec 16, 2015
@jkbradley
Copy link
Member Author

Merged with master

@jkbradley jkbradley deleted the pyspark-cv-seed branch December 16, 2015 23:13
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.

3 participants