Skip to content

Conversation

@yinxusen
Copy link
Contributor

What changes were proposed in this pull request?

https://issues.apache.org/jira/browse/SPARK-11892

Add save/load for spark ml.OneVsRest and its model. Also add OneVsRest and OneVsRestModel in MetaAlgorithmReadWrite.

How was this patch tested?

Test with Scala unit test.

@SparkQA
Copy link

SparkQA commented Nov 24, 2015

Test build #46604 has finished for PR 9934 at commit d24dcf8.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):\n * trait ClassifierTypeTrait\n

@yinxusen
Copy link
Contributor Author

Updated for 2.0 cc @jkbradley

@SparkQA
Copy link

SparkQA commented Jan 11, 2016

Test build #49131 has finished for PR 9934 at commit 4996e2f.

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

@jkbradley
Copy link
Member

I'll take a look now

@jkbradley
Copy link
Member

@yinxusen Actually, should I wait to review this until #9971 gets merged? Which should come first?

@SparkQA
Copy link

SparkQA commented Mar 23, 2016

Test build #2671 has finished for PR 9934 at commit 4996e2f.

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

@yinxusen
Copy link
Contributor Author

I'll update this after #9971 merged.

@yinxusen
Copy link
Contributor Author

Refactoring this one now.

@SparkQA
Copy link

SparkQA commented Mar 29, 2016

Test build #54411 has finished for PR 9934 at commit 2e31b85.

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

@yinxusen
Copy link
Contributor Author

@jkbradley Ready to review.

@jkbradley
Copy link
Member

Github says this PR does not conflict with master, but when I rebase on master, it has conflicts. Does this happen to you?

@yinxusen
Copy link
Contributor Author

Not happen on me. Maybe you can hold off first, I can check it later today because I am off my PC now, then ping you if it passes.

@jkbradley
Copy link
Member

Sure, I'll go ahead and review though. Btw, can you add the "[ML]" tag to the PR title?

@yinxusen
Copy link
Contributor Author

Sure

Sent from my iPhone

On Mar 29, 2016, at 14:59, jkbradley [email protected] wrote:

Sure, I'll go ahead and review though. Btw, can you add the "[ML]" tag to the PR title?


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub

@yinxusen yinxusen changed the title [SPARK-11892] Model export/import for spark.ml: OneVsRest [SPARK-11892][ML] Model export/import for spark.ml: OneVsRest Mar 30, 2016
@yinxusen
Copy link
Contributor Author

@jkbradley I merged with master, it seems good.

@SparkQA
Copy link

SparkQA commented Mar 30, 2016

Test build #54488 has finished for PR 9934 at commit e6b16a8.

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

case stage: MLWritable => // good
case other =>
throw new UnsupportedOperationException("OneVsRest write will fail " +
s" because it contains $name which does not implement Writable." +
Copy link
Member

Choose a reason for hiding this comment

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

"Writable" --> "MLWritable"

@Since("1.4.0") override val uid: String,
@Since("1.4.0") labelMetadata: Metadata,
@Since("1.4.0") override val uid: String,
@Since("1.4.0") val labelMetadata: Metadata,
Copy link
Member

Choose a reason for hiding this comment

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

This should be private[ml]. The Since tag was a mistake and can be removed since it does not show up in the public API.

@jkbradley
Copy link
Member

I think that should be it.

@SparkQA
Copy link

SparkQA commented Mar 31, 2016

Test build #54570 has finished for PR 9934 at commit 028e6fa.

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

@yinxusen
Copy link
Contributor Author

@jkbradley All fixed.

@jkbradley
Copy link
Member

LGTM
Merging with master
Thanks @yinxusen !

@yinxusen
Copy link
Contributor Author

Thanks!

@asfgit asfgit closed this in 8b207f3 Mar 31, 2016
@jkbradley
Copy link
Member

I made a JIRA for the Python API: https://issues.apache.org/jira/browse/SPARK-14306

@yinxusen
Copy link
Contributor Author

Sure I can take it.

Sent from my iPhone

On Mar 31, 2016, at 11:21, jkbradley [email protected] wrote:

I'll make a JIRA for the Python API


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub

@jkbradley
Copy link
Member

Thanks!

@yinxusen
Copy link
Contributor Author

@jkbradley It seems that OneVsRest is still missing for PySpark. I find the closed PR: #6403. I can help re-submitting it with the PySpark one.

@jkbradley
Copy link
Member

@yinxusen Thanks! That would be great.

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