Skip to content

[SPARK-16249][ML] Change visibility of Object ml.clustering.LDA to public for loading #13941

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.

Sign up for GitHub

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

Closed
hhbyyh wants to merge 4 commits into from
Closed

[SPARK-16249][ML] Change visibility of Object ml.clustering.LDA to public for loading #13941

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
43ce219
public lda load
YY-OnCall Jun 28, 2016
5131668
Merge remote-tracking branch 'upstream/master' into ldapublic
YY-OnCall Jun 30, 2016
11a077c
style and since
YY-OnCall Jun 30, 2016
6d3fec6
bump up version for load
hhbyyh Jul 6, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions mllib/src/main/scala/org/apache/spark/ml/clustering/LDA.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -880,11 +880,13 @@ class LDA @Since("1.6.0") (
}
}


private[clustering] object LDA extends DefaultParamsReadable[LDA] {
@Since("2.0.0")
object LDA extends DefaultParamsReadable[LDA] {
Copy link
Contributor

@yanboliang yanboliang Jun 29, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

@SInCE("2.0.0")

Copy link
Contributor

@yanboliang yanboliang Jun 30, 2016

Choose a reason for hiding this comment

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

For discussion: Should we bump up the version at L899 to 2.0.0? @Since is used for public function and LDA.load become public since 2.0.0 actually. cc @mengxr @jkbradley @MLnick

Copy link
Contributor

@MLnick MLnick Jul 5, 2016

Choose a reason for hiding this comment

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

yeah, seems we have generally had load be since the version that model loading was actually added for that model? e.g. I see that is the case for tree models etc

Copy link
Member

@jkbradley jkbradley Jul 5, 2016

Choose a reason for hiding this comment

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

Yes, please, bump to 2.0.0


/** Get dataset for spark.mllib LDA */
def getOldDataset(dataset: Dataset[_], featuresCol: String): RDD[(Long, OldVector)] = {
private[clustering] def getOldDataset(
dataset: Dataset[_],
featuresCol: String): RDD[(Long, OldVector)] = {
dataset
.withColumn("docId", monotonicallyIncreasingId())
.select("docId", featuresCol)
Expand All @@ -894,6 +896,6 @@ private[clustering] object LDA extends DefaultParamsReadable[LDA] {
}
}

@Since("1.6.0")
@Since("2.0.0")
override def load(path: String): LDA = super.load(path)
}