Skip to content
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.

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

[SPARK-15788][PYSPARK][ML] PySpark IDFModel missing "idf" property #13540

Closed
wants to merge 2 commits into from

Conversation

zjffdu
Copy link
Contributor

@zjffdu zjffdu commented Jun 7, 2016

What changes were proposed in this pull request?

add method idf to IDF in pyspark

How was this patch tested?

add unit test

@SparkQA
Copy link

SparkQA commented Jun 7, 2016

Test build #60116 has finished for PR 13540 at commit e5c11da.

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

@BryanCutler
Copy link
Member

Good catch! Not too big of deal, but should this be a property? Otherwise, LGTM
cc @MLnick

@MechCoder
Copy link
Contributor

LGTM as well. pending the nitpick by @BryanCutler

Not related, but it's been a while since I hacked on Spark or PySpark but at some point do we need better docs for PySpark? I couldn't figure out how the IDF's are calculated without looking at the Scala documentation.

@MLnick
Copy link
Contributor

MLnick commented Jun 7, 2016

We should make it a property
On Tue, 7 Jun 2016 at 11:28, Manoj Kumar notifications@github.com wrote:

LGTM as well. pending the nitpick by @BryanCutler
https://github.com/BryanCutler

Not related, but it's been a while since I hacked on Spark or PySpark but
at some point do we need better docs for PySpark? I couldn't figure out how
the IDF's are calculated without looking at the Scala documentation.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#13540 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AA_SB_BQRxelQdfaS3wy5ld9p8efsmwnks5qJbhpgaJpZM4IvqPP
.

@MLnick
Copy link
Contributor

MLnick commented Jun 7, 2016

Also, yes we should actually look at copying docs from Scala side where
appropriate. We can add that to the QA JIRAs for Python doc.

On Tue, 7 Jun 2016 at 11:28, Manoj Kumar notifications@github.com wrote:

LGTM as well. pending the nitpick by @BryanCutler
https://github.com/BryanCutler

Not related, but it's been a while since I hacked on Spark or PySpark but
at some point do we need better docs for PySpark? I couldn't figure out how
the IDF's are calculated without looking at the Scala documentation.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#13540 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AA_SB_BQRxelQdfaS3wy5ld9p8efsmwnks5qJbhpgaJpZM4IvqPP
.

@zjffdu
Copy link
Contributor Author

zjffdu commented Jun 8, 2016

Thanks @BryanCutler @MechCoder @MLnick for the review. I just update the PR to make it as property. Regarding the pyspark docs, I think there's umbrella jira to parity scala mllib and pyspark mllib, we can create sub task there.

@SparkQA
Copy link

SparkQA commented Jun 8, 2016

Test build #60145 has finished for PR 13540 at commit d1c00da.

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

@MLnick
Copy link
Contributor

MLnick commented Jun 9, 2016

LGTM

@asfgit asfgit closed this in e594b49 Jun 9, 2016
asfgit pushed a commit that referenced this pull request Jun 9, 2016
## What changes were proposed in this pull request?

add method idf to IDF in pyspark

## How was this patch tested?

add unit test

Author: Jeff Zhang <zjffdu@apache.org>

Closes #13540 from zjffdu/SPARK-15788.

(cherry picked from commit e594b49)
Signed-off-by: Nick Pentreath <nickp@za.ibm.com>
zjffdu added a commit to zjffdu/spark that referenced this pull request Jun 10, 2016
## What changes were proposed in this pull request?

add method idf to IDF in pyspark

## How was this patch tested?

add unit test

Author: Jeff Zhang <zjffdu@apache.org>

Closes apache#13540 from zjffdu/SPARK-15788.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants