Skip to content

[SPARK-18613][ML] make spark.mllib LDA dependencies in spark.ml LDA private#16860

Closed
sueann wants to merge 1 commit intoapache:masterfrom
sueann:SPARK-18613
Closed

[SPARK-18613][ML] make spark.mllib LDA dependencies in spark.ml LDA private#16860
sueann wants to merge 1 commit intoapache:masterfrom
sueann:SPARK-18613

Conversation

@sueann
Copy link
Contributor

@sueann sueann commented Feb 8, 2017

What changes were proposed in this pull request?

spark.ml.*LDAModel classes were exposing spark.mllib LDA models via protected methods. Made them package (clustering) private.

How was this patch tested?

build/sbt doc  # "millib.clustering" no longer appears in the docs for *LDA* classes
build/sbt compile  # compiles
build/sbt
> mllib/testOnly   # tests pass

@srowen
Copy link
Member

srowen commented Feb 8, 2017

I get it, but making something less visible after it's released is also a breaking change. Without a good reason for that we shouldn't make changes like this.

@jkbradley
Copy link
Member

@srowen This actually isn't a breaking change. This is making protected methods private within classes which cannot be extended outside of Spark. The classes are not final, but they only provide private constructors.

@srowen
Copy link
Member

srowen commented Feb 8, 2017

Oh, the class is private already, right on. That seems OK then.

@jkbradley
Copy link
Member

jkbradley commented Feb 8, 2017

@sueann Try running dev/mima locally. I bet you'll have to add stuff to the MiMaExcludes.scala file b/c of false positives.

@srowen Yep, I'm sometimes thankful for our paranoia about making things too public!

@SparkQA
Copy link

SparkQA commented Feb 8, 2017

Test build #72607 has finished for PR 16860 at commit ce8abb3.

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

@jkbradley
Copy link
Member

LGTM
Merging with master
Thanks @sueann !

@asfgit asfgit closed this in 3a43ae7 Feb 10, 2017
cmonkey pushed a commit to cmonkey/spark that referenced this pull request Feb 15, 2017
…rivate

## What changes were proposed in this pull request?
spark.ml.*LDAModel classes were exposing spark.mllib LDA models via protected methods. Made them package (clustering) private.

## How was this patch tested?
```
build/sbt doc  # "millib.clustering" no longer appears in the docs for *LDA* classes
build/sbt compile  # compiles
build/sbt
> mllib/testOnly   # tests pass
```

Author: sueann <sueann@databricks.com>

Closes apache#16860 from sueann/SPARK-18613.
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.

5 participants