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-10264][Documentation, ML] Added Since annotation for ml.recomendation #8532

Closed
wants to merge 8 commits into from

Conversation

tijoparacka
Copy link
Contributor

Added Since annotation for ALS.scala

@@ -255,64 +262,81 @@ class ALSModel private[ml] (
* preferences rather than explicit ratings given to items.
*/
@Experimental
class ALS(override val uid: String) extends Estimator[ALSModel] with ALSParams {
@Since("1.3.0")
class ALS(@Since("1.4.0") override val uid: String) extends Estimator[ALSModel] with ALSParams {

import org.apache.spark.ml.recommendation.ALS.Rating

def this() = this(Identifiable.randomUID("als"))
Copy link
Contributor

Choose a reason for hiding this comment

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

Please annotate auxillary constructor

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added Auxiliary constructor

@feynmanliang
Copy link
Contributor

Made a pass

@feynmanliang
Copy link
Contributor

LGTM

@@ -178,6 +178,7 @@ private[recommendation] trait ALSParams extends ALSModelParams with HasMaxIter w
* @param itemFactors a DataFrame that stores item factors in two columns: `id` and `features`
*/
@Experimental
@Since("1.3.0")
class ALSModel private[ml] (
override val uid: String,
Copy link
Contributor

Choose a reason for hiding this comment

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

Document public params in the constructor. Though the constructor is private, those variables are public.

    @Since("1.4.0") override val uid: String,
    @Since("1.4.0") override val rank: Int

@tijoparacka
Copy link
Contributor Author

Any of you can review this and merge. I may loose track if it is delayed more.

@mengxr
Copy link
Contributor

mengxr commented Oct 20, 2015

ok to test

@@ -355,7 +383,11 @@ object ALS extends Logging {
* Rating class for better code readability.
*/
@DeveloperApi
case class Rating[@specialized(Int, Long) ID](user: ID, item: ID, rating: Float)
@Since("1.3.0")
Copy link
Contributor

Choose a reason for hiding this comment

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

We don't need @Since annotation for developer APIs.

@SparkQA
Copy link

SparkQA commented Oct 21, 2015

Test build #44015 has finished for PR 8532 at commit 7b65e62.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):\n * class ALS(@Since(\"1.4.0\") override val uid: String) extends Estimator[ALSModel] with ALSParams\n * case class Rating[@specialized(Int, Long) ID] @Since(\"1.3.0\")(\n

@jkbradley
Copy link
Member

@tijoparacka Ping! Could you please update this based on @mengxr 's comments and also fix the merge conflicts? Thank you!

@srowen
Copy link
Member

srowen commented Jan 13, 2016

Close this PR please @tijoparacka if you're not updating it

@mengxr
Copy link
Contributor

mengxr commented Jan 25, 2016

I closed this PR. The feature is implemented in #10756.

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