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-22734][ML][PySpark] Added Python API for VectorSizeHint. #20112

Closed
wants to merge 3 commits into from

Conversation

MrBago
Copy link
Contributor

@MrBago MrBago commented Dec 29, 2017

(Please fill in changes proposed in this fix)

Python API for VectorSizeHint Transformer.

(Please explain how this patch was tested. E.g. unit tests, integration tests, manual tests)

doc-tests.

@SparkQA
Copy link

SparkQA commented Dec 29, 2017

Test build #85496 has finished for PR 20112 at commit 83bb7de.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
  • class VectorSizeHint(JavaTransformer, HasInputCol, HasHandleInvalid, JavaMLReadable,

Copy link
Member

@jkbradley jkbradley left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

You'll need to add this class to the __all__ list at the top of the file.

Since VectorSizeHint modifies the inputCol, rather than creating a new outputCol, we should explicitly document this. Would you mind adding a note to the class doc string in both Python and Scala? (It's OK to do both in this PR.)

self._set(size=value)

@keyword_only
def __init__(self, inputCol=None, size=None, handleInvalid="error"):
Copy link
Member

Choose a reason for hiding this comment

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

Let's stick with the order which all other python classes follow: dummy Params, init, Param setters & getters

@@ -3466,6 +3466,72 @@ def selectedFeatures(self):
return self._call_java("selectedFeatures")


@inherit_doc
class VectorSizeHint(JavaTransformer, HasInputCol, HasHandleInvalid, JavaMLReadable,
Copy link
Member

Choose a reason for hiding this comment

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

You'll need to override handleInvalid, like in the Scala API, since it takes different values & has a different docstring.

@SparkQA
Copy link

SparkQA commented Dec 29, 2017

Test build #85527 has finished for PR 20112 at commit 9a1ee2b.

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

@SparkQA
Copy link

SparkQA commented Dec 29, 2017

Test build #85528 has finished for PR 20112 at commit 1ec7a41.

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

@jkbradley
Copy link
Member

Looks good except for the style issue

@SparkQA
Copy link

SparkQA commented Dec 30, 2017

Test build #85537 has finished for PR 20112 at commit de9ea9f.

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

@jkbradley
Copy link
Member

LGTM
merging with master
Thank you!

@asfgit asfgit closed this in 8169630 Dec 30, 2017
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