Skip to content

[SPARK-5769] Set params in constructors and in setParams in Python ML pipelines #4564

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 8 commits into from
Closed

[SPARK-5769] Set params in constructors and in setParams in Python ML pipelines #4564

wants to merge 8 commits into from

Conversation

@mengxr
Copy link
Contributor

@mengxr mengxr commented Feb 12, 2015

This PR allow Python users to set params in constructors and in setParams, where we use decorator keyword_only to force keyword arguments. The trade-off is discussed in the design doc of SPARK-4586.

Generated doc:
screen shot 2015-02-12 at 3 06 58 am

CC: @davies @rxin

@SparkQA
Copy link

SparkQA commented Feb 12, 2015

Test build #27347 has started for PR 4564 at commit c9384b8.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Feb 12, 2015

Test build #27347 has finished for PR 4564 at commit c9384b8.

  • This patch fails Python style tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@AmplabJenkins
Copy link

AmplabJenkins commented Feb 12, 2015

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/27347/
Test FAILed.

@SparkQA
Copy link

SparkQA commented Feb 12, 2015

Test build #27348 has started for PR 4564 at commit fdde5fc.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Feb 12, 2015

Test build #27348 has finished for PR 4564 at commit fdde5fc.

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

@AmplabJenkins
Copy link

AmplabJenkins commented Feb 12, 2015

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/27348/
Test PASSed.

@SparkQA
Copy link

SparkQA commented Feb 13, 2015

Test build #27424 has started for PR 4564 at commit 5032097.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Feb 13, 2015

Test build #27424 has finished for PR 4564 at commit 5032097.

  • This patch fails Python style tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@AmplabJenkins
Copy link

AmplabJenkins commented Feb 13, 2015

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/27424/
Test FAILed.

@mengxr
Copy link
Contributor Author

mengxr commented Feb 13, 2015

@davies I updated the constructor doc signature. Auto-completion works in both iPython and IntelliJ now. The only issue is that the help doc in iPython still contains the doc signature. I think this is a minor one.

@SparkQA
Copy link

SparkQA commented Feb 13, 2015

Test build #27425 has started for PR 4564 at commit cbc15d3.

  • This patch merges cleanly.

davies
davies reviewed Feb 13, 2015
View reviewed changes
python/pyspark/ml/classification.py Outdated
Copy link
Contributor

@davies davies Feb 13, 2015

Choose a reason for hiding this comment

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

Wait for #4557, then you could user new API for convert RDD into DataFrame:

dataset = rdd.toDF()

Copy link
Contributor

@davies davies Feb 15, 2015

Choose a reason for hiding this comment

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

#4557 is closed, so you change update the tests (examples) to use new API.

@SparkQA
Copy link

SparkQA commented Feb 13, 2015

Test build #27425 has finished for PR 4564 at commit cbc15d3.

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

@AmplabJenkins
Copy link

AmplabJenkins commented Feb 13, 2015

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/27425/
Test PASSed.

@SparkQA
Copy link

SparkQA commented Feb 15, 2015

Test build #27530 has started for PR 4564 at commit fedf720.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Feb 15, 2015

Test build #27530 has finished for PR 4564 at commit fedf720.

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

@AmplabJenkins
Copy link

AmplabJenkins commented Feb 15, 2015

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/27530/
Test PASSed.

@davies
Copy link
Contributor

davies commented Feb 15, 2015

LGTM， thanks！

asfgit pushed a commit that referenced this pull request Feb 16, 2015
@mengxr
[SPARK-5769] Set params in constructors and in setParams in Python ML…
d710991 
… pipelines

This PR allow Python users to set params in constructors and in setParams, where we use decorator `keyword_only` to force keyword arguments. The trade-off is discussed in the design doc of SPARK-4586.

Generated doc:
![screen shot 2015-02-12 at 3 06 58 am](https://cloud.githubusercontent.com/assets/829644/6166491/9cfcd06a-b265-11e4-99ea-473d866634fc.png)

CC: davies rxin

Author: Xiangrui Meng <[email protected]>

Closes #4564 from mengxr/py-pipeline-kw and squashes the following commits:

fedf720 [Xiangrui Meng] use toDF
d565f2c [Xiangrui Meng] Merge remote-tracking branch 'apache/master' into py-pipeline-kw
cbc15d3 [Xiangrui Meng] fix style
5032097 [Xiangrui Meng] update pipeline signature
950774e [Xiangrui Meng] simplify keyword_only and update constructor/setParams signatures
fdde5fc [Xiangrui Meng] fix style
c9384b8 [Xiangrui Meng] fix sphinx doc
8e59180 [Xiangrui Meng] add setParams and make constructors take params, where we force keyword args

(cherry picked from commit cd4a153)
Signed-off-by: Xiangrui Meng <[email protected]>
@mengxr
Copy link
Contributor Author

mengxr commented Feb 16, 2015

Merged into master and branch-1.3.

@asfgit asfgit closed this in cd4a153 Feb 16, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@mengxr @SparkQA @AmplabJenkins @davies