Skip to content

[SPARK-12780] Inconsistency returning value of ML python models' properties - #10724

Closed
yinxusen wants to merge 6 commits into
apache:masterfrom
yinxusen:SPARK-12780
Closed

[SPARK-12780] Inconsistency returning value of ML python models' properties#10724
yinxusen wants to merge 6 commits into
apache:masterfrom
yinxusen:SPARK-12780

Conversation

@yinxusen

Copy link
Copy Markdown
Contributor

@SparkQA

Copy link
Copy Markdown

Test build #49244 has finished for PR 10724 at commit 993dc9c.

  • This patch fails PySpark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@yinxusen

Copy link
Copy Markdown
ContributorAuthor

test it please

@SparkQA

Copy link
Copy Markdown

Test build #49313 has finished for PR 10724 at commit becc49c.

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

@yinxusen

Copy link
Copy Markdown
ContributorAuthor

retest this please

@SparkQA

Copy link
Copy Markdown

Test build #49314 has finished for PR 10724 at commit becc49c.

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

@yinxusen

Copy link
Copy Markdown
ContributorAuthor

Something wrong with Jenkins?

@jkbradley

Copy link
Copy Markdown
Member

A PR broke the build, but it should work now.

@jkbradley

Copy link
Copy Markdown
Member

The feature.py changes look fine to me.

@davies Could you please glance at the serialization change to confirm it's OK? Thanks!

@SparkQA

Copy link
Copy Markdown

Test build #2379 has finished for PR 10724 at commit becc49c.

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

@yinxusen

Copy link
Copy Markdown
ContributorAuthor

@jkbradley@holdenk As I mentioned here #10186, we can work around the Ser/De issue with type conversion. So same solution here, I remove the Scala side code. But I still think there is a problem when ser/deing Scala Array. How about we merge this first then file another PR to fix it?

@yinxusen

Copy link
Copy Markdown
ContributorAuthor

@SparkQA

Copy link
Copy Markdown

Test build #49454 has finished for PR 10724 at commit f5623b6.

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

@jkbradley

Copy link
Copy Markdown
Member

@yinxusen I spoke with @davies and I think it would be best to use _call_java wherever possible for simplicity. Otherwise, developers have to remember to put list() around the JVM methods they call.

Since you have created [https://github.com//pull/10772], let's use it to create the fix. @davies said your original fix (in Scala in [https://github.com/yinxusen/spark/commit/c5a170da04f9941f4515f3263939bab9886c2c85]) seemed reasonable, so perhaps use that if your new one is not working. After that gets merged, we can return to this and the other PRs.

@yinxusen

Copy link
Copy Markdown
ContributorAuthor

retest it please

@SparkQA

Copy link
Copy Markdown

Test build #50111 has finished for PR 10724 at commit c0cf68c.

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

@yinxusen

Copy link
Copy Markdown
ContributorAuthor

Hi @jkbradley, this is the follow-up PR of #10772, shall we merge it?

@jkbradley

Copy link
Copy Markdown
Member

LGTM
I'll merge this with master
Thanks!

jkbradley pushed a commit to jkbradley/spark that referenced this pull request Jan 27, 2016
asfgit pushed a commit that referenced this pull request Feb 2, 2016
…ML python models' properties
Backport of [SPARK-12780] for branch-1.6
Original PR for master: #10724
This fixes StringIndexerModel.labels in pyspark.
Author: Xusen Yin <yinxusen@gmail.com>
Closes#10950 from jkbradley/yinxusen-spark-12780-backport.
Sign up for freeto 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

@yinxusen@SparkQA@jkbradley