[SPARK-10259] [ML] Add @since annotation to ml.classification #8534

ghost · 2015-08-31T09:01:23Z

Add @SInCE annotation to ml.classification

feynmanliang · 2015-08-31T22:04:05Z

@taishi-oss these were already in spark prior to 1.6, please see the parent JIRA for how to track down the correct version number to use in the @since annotations.

ghost · 2015-09-08T09:07:02Z

Sorry for being late.
I modified @SInCE annotations.

feynmanliang · 2015-09-08T16:09:48Z

mllib/src/main/scala/org/apache/spark/ml/classification/DecisionTreeClassifier.scala

@@ -35,6 +35,8 @@ import org.apache.spark.sql.DataFrame
 * for classification.
 * It supports both binary and multiclass labels, as well as both continuous and categorical
 * features.
+ *
+ * @since 1.4.0
 */
 @Experimental
 final class DecisionTreeClassifier(override val uid: String)


Please also annotate override constructor fields, see ml.KMeans

feynmanliang · 2015-09-08T16:13:41Z

I only pointed out some issues in the first few files, do you mind fixing them in all the files?

ghost · 2015-09-10T06:49:25Z

I'm sorry for taking your time.
I modified the code that was pointed out.

feynmanliang · 2015-09-10T16:53:40Z

@taishi-oss no worries, thank you for your help! reviewing now

feynmanliang · 2015-09-10T17:04:51Z

Not all methods in a file were introduced in the same version (e.g. see my comment about DecisionTreeClassifer.copy). Can you make sure that the annotation versions are correct? the jira has directions on how to identify the version a particular method was introduced.

ghost · 2015-09-14T10:05:57Z

Sorry, I misunderstood.
I check when all methods were introduced and modify it.
(use SPARK-7751 as a reference.)

If it is not my mistake, DecisionTreeClassifer.copy was introduced in 1.4.1.
And Should I add @SInCE annotation to val?

feynmanliang · 2015-09-14T19:25:11Z

I believe the copy overrides were added by this commit which went in to 1.5, not 1.4.1. SPARK-7751 has detailed directions on how to use the git history to identify the correct version numbers. You could also look at git blames and histories to figure out the exact commit that introduced it.

you should add annotations to all public methods and constructor parameters (i.e. to vals if they are parameters in a public constructor).

ghost · 2015-09-15T09:17:34Z

I think that commit is right.
but "git show" command results as follows.
I also think this is right.

[takahashi@phi spark]$ git show v1.4.0:mllib/src/main/scala/org/apache/spark/ml/classification/DecisionTreeClassifier.scala | grep "override def copy(extra: ParamMap): DecisionTreeClassifier = defaultCopy(extra)"
[takahashi@phi spark]$ git show v1.4.1:mllib/src/main/scala/org/apache/spark/ml/classification/DecisionTreeClassifier.scala | grep "override def copy(extra: ParamMap): DecisionTreeClassifier = defaultCopy(extra)"
  override def copy(extra: ParamMap): DecisionTreeClassifier = defaultCopy(extra)
[takahashi@phi spark]$ git show v1.5.0:mllib/src/main/scala/org/apache/spark/ml/classification/DecisionTreeClassifier.scala | grep "override def copy(extra: ParamMap): DecisionTreeClassifier = defaultCopy(extra)"
  override def copy(extra: ParamMap): DecisionTreeClassifier = defaultCopy(extra)

Which should I believe?

feynmanliang · 2015-09-15T16:20:01Z

Hmm I actually don't know the answer to that, maybe @mengxr can help

ghost · 2015-09-16T09:00:56Z

@mengxr Could you take a look?

Now, I'm adding Since annotation to ml.classification.
I think DecisionTreeClassifer.copy is introduced in this commit (v.1.5.0).
but, "git show" command shows that DecisionTreeClassifer.copy is introduce in v1.4.1.
(Please see above.)

Which version should I add Since annotation to this method?

ghost · 2015-09-17T08:45:29Z

I think DecisionTreeClassifer.copy is introduced in v1.5.0.
but, This method is brought into v.1.4.1 using "git-cherry-pick".
So "git blames" says v.1.5.0 but "git show" says v.1.4.1.

I compare "git show" commnad's results with "git blames"'s results in ml.classification, and then there are some differnces as desribed above.

yu-iskw · 2015-11-01T21:46:46Z

@Hiross thank you for your contribution. Could you update this PR? After that, I could review it.

yu-iskw · 2015-11-02T00:28:55Z

Jenkins, test this please.

ghost · 2015-11-04T07:19:53Z

Sorry for being late.
I rebased this branch.

yu-iskw · 2015-11-06T01:19:02Z

mllib/src/main/scala/org/apache/spark/ml/classification/DecisionTreeClassifier.scala

-    override val rootNode: Node,
-    override val numFeatures: Int,
-    override val numClasses: Int)
+    @Since("1.5.0")override val uid: String,


since 1.4.0

yu-iskw · 2015-11-06T01:20:11Z

@taishi-oss thanks for rebaseing master. Could you add @Since to all public variables?

yu-iskw · 2015-11-12T17:52:31Z

ping @taishi-oss

ghost · 2015-11-13T08:19:48Z

@yu-iskw Sorry for being late, I'm do right now.

sarutak · 2015-12-03T17:59:54Z

mllib/src/main/scala/org/apache/spark/ml/classification/LogisticRegression.scala

  extends ProbabilisticClassifier[Vector, LogisticRegression, LogisticRegressionModel]
  with LogisticRegressionParams with DefaultParamsWritable with Logging {

+  @Since("1.4.0")


"git show" says it was introduced in v.1.4.0.

hiro [spark] (master) > git show v1.2.0:mllib/src/main/scala/org/apache/spark/ml/classification/LogisticRegression.scala | grep "def this" hiro [spark] (master) > git show v1.3.0:mllib/src/main/scala/org/apache/spark/ml/classification/LogisticRegression.scala | grep "def this" hiro [spark] (master) > git show v1.4.0:mllib/src/main/scala/org/apache/spark/ml/classification/LogisticRegression.scala | grep "def this" def this() = this(Identifiable.randomUID("logreg"))

sarutak · 2015-12-03T18:15:42Z

I've left some comments, otherwise it's LGTM but It's good if @jkbradley or @mengxr can take a look at.

sarutak · 2015-12-03T18:22:09Z

Jenkins, test this please.

SparkQA · 2015-12-03T19:13:08Z

Test build #47152 has finished for PR 8534 at commit 4e0a925.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):\n * final class DecisionTreeClassifier (\n * final class GBTClassifier(\n * class LogisticRegression(\n * class MultilayerPerceptronClassifier(\n * class NaiveBayes(\n * final class OneVsRest(\n * final class RandomForestClassifier(\n

SparkQA · 2015-12-03T20:38:44Z

Test build #47153 has finished for PR 8534 at commit 4e0a925.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):\n * final class DecisionTreeClassifier (\n * final class GBTClassifier(\n * class LogisticRegression(\n * class MultilayerPerceptronClassifier(\n * class NaiveBayes(\n * final class OneVsRest(\n * final class RandomForestClassifier(\n

ghost · 2015-12-04T11:12:37Z

Thank you for reviewing.
I'm modifying.

yu-iskw · 2015-12-04T11:37:28Z

@sarutak thank you for your help!

SparkQA · 2015-12-04T12:11:33Z

Test build #47196 has finished for PR 8534 at commit 3743123.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):\n * final class DecisionTreeClassifier (\n * final class GBTClassifier(\n * class LogisticRegression(\n * class MultilayerPerceptronClassifier(\n * class NaiveBayes(\n * final class OneVsRest(\n * final class RandomForestClassifier(\n

ghost · 2015-12-07T07:52:48Z

I modified the code that was pointed out.
@sarutak @yu-iskw Thank you for reviewing!

SparkQA · 2015-12-07T08:32:44Z

Test build #47260 has finished for PR 8534 at commit 6831e7a.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):\n * final class DecisionTreeClassifier @Since(\"1.4.0\") (\n * final class GBTClassifier @Since(\"1.4.0\") (\n * class LogisticRegression @Since(\"1.2.0\") (\n * class MultilayerPerceptronClassifier @Since(\"1.5.0\") (\n * class NaiveBayes @Since(\"1.5.0\") (\n * final class OneVsRest @Since(\"1.4.0\") (\n * final class RandomForestClassifier @Since(\"1.4.0\") (\n * public abstract static class PrefixComputer\n

mengxr · 2015-12-08T07:47:24Z

Merged into master and branch-1.6. Thanks!

Add since annotation to ml.classification Author: Takahashi Hiroshi <[email protected]> Closes #8534 from taishi-oss/issue10259. (cherry picked from commit 7d05a62) Signed-off-by: Xiangrui Meng <[email protected]>

ghost · 2015-12-08T08:54:46Z

Thank you for merging!

feynmanliang reviewed Sep 8, 2015
View reviewed changes

yu-iskw reviewed Nov 6, 2015
View reviewed changes

sarutak reviewed Dec 3, 2015
View reviewed changes

Takahashi Hiroshi added 2 commits December 7, 2015 16:53

Add @SInCE annotation to ml.classification

b107213

modify my miss

6831e7a

asfgit closed this in 7d05a62 Dec 8, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-10259] [ML] Add @since annotation to ml.classification #8534

[SPARK-10259] [ML] Add @since annotation to ml.classification #8534

ghost commented Aug 31, 2015

feynmanliang commented Aug 31, 2015

ghost commented Sep 8, 2015

feynmanliang Sep 8, 2015

feynmanliang commented Sep 8, 2015

ghost commented Sep 10, 2015

feynmanliang commented Sep 10, 2015

feynmanliang commented Sep 10, 2015

ghost commented Sep 14, 2015

feynmanliang commented Sep 14, 2015

ghost commented Sep 15, 2015

feynmanliang commented Sep 15, 2015

ghost commented Sep 16, 2015

ghost commented Sep 17, 2015

yu-iskw commented Nov 1, 2015

yu-iskw commented Nov 2, 2015

ghost commented Nov 4, 2015

yu-iskw Nov 6, 2015

yu-iskw commented Nov 6, 2015

yu-iskw commented Nov 12, 2015

ghost commented Nov 13, 2015

sarutak Dec 3, 2015

ghost Dec 4, 2015

sarutak commented Dec 3, 2015

sarutak commented Dec 3, 2015

SparkQA commented Dec 3, 2015

SparkQA commented Dec 3, 2015

ghost commented Dec 4, 2015

yu-iskw commented Dec 4, 2015

SparkQA commented Dec 4, 2015

ghost commented Dec 7, 2015

SparkQA commented Dec 7, 2015

mengxr commented Dec 8, 2015

ghost commented Dec 8, 2015

[SPARK-10259] [ML] Add @since annotation to ml.classification #8534

[SPARK-10259] [ML] Add @since annotation to ml.classification #8534

Conversation

ghost commented Aug 31, 2015

feynmanliang commented Aug 31, 2015

ghost commented Sep 8, 2015

feynmanliang Sep 8, 2015

Choose a reason for hiding this comment

feynmanliang commented Sep 8, 2015

ghost commented Sep 10, 2015

feynmanliang commented Sep 10, 2015

feynmanliang commented Sep 10, 2015

ghost commented Sep 14, 2015

feynmanliang commented Sep 14, 2015

ghost commented Sep 15, 2015

feynmanliang commented Sep 15, 2015

ghost commented Sep 16, 2015

ghost commented Sep 17, 2015

yu-iskw commented Nov 1, 2015

yu-iskw commented Nov 2, 2015

ghost commented Nov 4, 2015

yu-iskw Nov 6, 2015

Choose a reason for hiding this comment

yu-iskw commented Nov 6, 2015

yu-iskw commented Nov 12, 2015

ghost commented Nov 13, 2015

sarutak Dec 3, 2015

Choose a reason for hiding this comment

ghost Dec 4, 2015

Choose a reason for hiding this comment

sarutak commented Dec 3, 2015

sarutak commented Dec 3, 2015

SparkQA commented Dec 3, 2015

SparkQA commented Dec 3, 2015

ghost commented Dec 4, 2015

yu-iskw commented Dec 4, 2015

SparkQA commented Dec 4, 2015

ghost commented Dec 7, 2015

SparkQA commented Dec 7, 2015

mengxr commented Dec 8, 2015

ghost commented Dec 8, 2015