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-29867][ML][PYTHON] Add __repr__ in Python ML Models #26489

Closed
wants to merge 3 commits into from

Conversation

huaxingao
Copy link
Contributor

What changes were proposed in this pull request?

Add __repr__ in Python ML Models

Why are the changes needed?

In Python ML Models, some of them have __repr__, others don't. In the doctest, when calling Model.setXXX, some of the Models print out the xxxModel... correctly, some of them can't because of lacking the __repr__ method. For example:

    >>> gm = GaussianMixture(k=3, tol=0.0001, seed=10)
    >>> model = gm.fit(df)
    >>> model.setPredictionCol("newPrediction")
    GaussianMixture...

After the change, the above code will become the following:

    >>> gm = GaussianMixture(k=3, tol=0.0001, seed=10)
    >>> model = gm.fit(df)
    >>> model.setPredictionCol("newPrediction")
    GaussianMixtureModel...

Does this PR introduce any user-facing change?

Yes.

How was this patch tested?

doctest

@huaxingao huaxingao changed the title [SPARK-29876][ML][PYTHON] Add __repr__ in Python ML Models [SPARK-29867][ML][PYTHON] Add __repr__ in Python ML Models Nov 12, 2019
@SparkQA
Copy link

SparkQA commented Nov 12, 2019

Test build #113651 has finished for PR 26489 at commit a6c649a.

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

@@ -356,6 +356,9 @@ def intercept(self):
"""
return self._call_java("intercept")

def __repr__(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we put this method to some common class (like JavaModel) and add __repr__ only in classes not extending JavaModel(like OneVsRestModel) ?

@SparkQA
Copy link

SparkQA commented Nov 14, 2019

Test build #113797 has finished for PR 26489 at commit c9b5e28.

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

@SparkQA
Copy link

SparkQA commented Nov 14, 2019

Test build #113799 has finished for PR 26489 at commit 6d97563.

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

Copy link
Member

@srowen srowen left a comment

Choose a reason for hiding this comment

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

Yep, good change. Looks OK pending tests.

Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

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

+1, LGTM. The common __repl__ is nice.
Thank you, @huaxingao , @zhengruifeng , @srowen .
Merged to master.

@huaxingao
Copy link
Contributor Author

Thanks! @dongjoon-hyun @srowen @zhengruifeng

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants