Skip to content

Conversation

@MLnick
Copy link
Contributor

@MLnick MLnick commented Jun 13, 2016

This PR adds missing @Since annotations to ml.feature package.

Closes #8505.

How was this patch tested?

Existing tests.

@MLnick MLnick force-pushed the add-since-annotations branch from 3c8be11 to bb43722 Compare June 13, 2016 11:46
@MLnick
Copy link
Contributor Author

MLnick commented Jun 13, 2016

cc @yanboliang @srowen @jkbradley

@SparkQA
Copy link

SparkQA commented Jun 13, 2016

Test build #60400 has finished for PR 13641 at commit 3c8be11.

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

@SparkQA
Copy link

SparkQA commented Jun 13, 2016

Test build #60401 has finished for PR 13641 at commit bb43722.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • final class Binarizer @Since(\"1.4.0\") (@Since(\"1.4.0\") override val uid: String)
    • final class Bucketizer @Since(\"1.4.0\") (@Since(\"1.4.0\") override val uid: String)
    • final class ChiSqSelector @Since(\"1.6.0\") (@Since(\"1.6.0\") override val uid: String)
    • class CountVectorizer @Since(\"1.5.0\") (@Since(\"1.5.0\") override val uid: String)
    • class CountVectorizerModel(
    • class DCT @Since(\"1.5.0\") (@Since(\"1.5.0\") override val uid: String)
    • class ElementwiseProduct @Since(\"1.4.0\") (@Since(\"1.4.0\") override val uid: String)
    • class HashingTF @Since(\"1.4.0\") (@Since(\"1.4.0\") override val uid: String)
    • final class IDF @Since(\"1.4.0\") (@Since(\"1.4.0\") override val uid: String)
    • class Interaction @Since(\"1.6.0\") (@Since(\"1.6.0\") override val uid: String) extends Transformer
    • class MaxAbsScaler @Since(\"2.0.0\") (@Since(\"2.0.0\") override val uid: String)
    • class MinMaxScaler @Since(\"1.5.0\") (@Since(\"1.5.0\") override val uid: String)
    • class NGram @Since(\"1.5.0\") (@Since(\"1.5.0\") override val uid: String)
    • class Normalizer @Since(\"1.4.0\") (@Since(\"1.4.0\") override val uid: String)
    • class OneHotEncoder @Since(\"1.4.0\") (@Since(\"1.4.0\") override val uid: String) extends Transformer
    • class PCA @Since(\"1.5.0\") (
    • class PolynomialExpansion @Since(\"1.4.0\") (@Since(\"1.4.0\") override val uid: String)
    • final class QuantileDiscretizer @Since(\"1.6.0\") (@Since(\"1.6.0\") override val uid: String)
    • class RFormula @Since(\"1.5.0\") (@Since(\"1.5.0\") override val uid: String)
    • class SQLTransformer @Since(\"1.6.0\") (@Since(\"1.6.0\") override val uid: String) extends Transformer
    • class StandardScaler @Since(\"1.2.0\") (
    • class StopWordsRemover @Since(\"1.5.0\") (@Since(\"1.5.0\") override val uid: String)
    • class StringIndexer @Since(\"1.4.0\") (
    • class Tokenizer @Since(\"1.2.0\") (@Since(\"1.4.0\") override val uid: String)
    • class RegexTokenizer @Since(\"1.4.0\") (@Since(\"1.4.0\") override val uid: String)
    • class VectorAssembler @Since(\"1.4.0\") (@Since(\"1.4.0\") override val uid: String)
    • class VectorIndexer @Since(\"1.4.0\") (
    • final class VectorSlicer @Since(\"1.5.0\") (@Since(\"1.5.0\") override val uid: String)
    • final class Word2Vec @Since(\"1.4.0\") (

@SparkQA
Copy link

SparkQA commented Jun 13, 2016

Test build #60402 has finished for PR 13641 at commit 9978afa.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • class StandardScaler @Since(\"1.4.0\") (
    • class Tokenizer @Since(\"1.4.0\") (@Since(\"1.4.0\") 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.

Not due to this PR, but here seems like a typo since MaxAbsScaler was added after 1.6.0.

Copy link
Contributor

Choose a reason for hiding this comment

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

And in L174 and L177 should also be @Since("2.0.0").

@yanboliang
Copy link
Contributor

@MLnick I found you did not add @Since for all params definition, is this as expected?I think we should add them.

@MLnick MLnick force-pushed the add-since-annotations branch from 9978afa to 86ab82b Compare June 17, 2016 08:15
@SparkQA
Copy link

SparkQA commented Jun 17, 2016

Test build #60694 has finished for PR 13641 at commit 86ab82b.

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

@MLnick
Copy link
Contributor Author

MLnick commented Jun 17, 2016

@yanboliang thanks - updated.

I also added annotations on overridden methods transformSchema and copy.

Let me know if I missed anything!

@SparkQA
Copy link

SparkQA commented Jun 17, 2016

Test build #60699 has finished for PR 13641 at commit 2fc676a.

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

@SparkQA
Copy link

SparkQA commented Jun 17, 2016

Test build #60700 has finished for PR 13641 at commit 8c24513.

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

* the vector to multiply with input vectors
* @group param
*/
@Since("1.4.0")
Copy link
Contributor

Choose a reason for hiding this comment

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

This uses the new Vector. So ideally it is since 2.0.0. I'm not sure about whether we should change the class since version to 2.0 as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good point, and also yeah technically all unary transformers over Vector should probably be since 2.0.0. Will update

@MLnick
Copy link
Contributor Author

MLnick commented Jun 20, 2016

@mengxr @yanboliang I went ahead and bumped all UnaryTransformer[Vector, ...] to 2.0.0 - since these classes are not binary compatible with the pre-2.0 version, it makes sense to me to do that. Also updated few vals in some of the model classes.

@SparkQA
Copy link

SparkQA commented Jun 20, 2016

Test build #60842 has finished for PR 13641 at commit 34b5f37.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • class ElementwiseProduct @Since(\"2.0.0\") (@Since(\"2.0.0\") override val uid: String)
    • class Normalizer @Since(\"2.0.0\") (@Since(\"2.0.0\") override val uid: String)
    • class PolynomialExpansion @Since(\"2.0.0\") (@Since(\"2.0.0\") override val uid: String)

@asfgit asfgit closed this in 37494a1 Jun 21, 2016
asfgit pushed a commit that referenced this pull request Jun 21, 2016
This PR adds missing `Since` annotations to `ml.feature` package.

Closes #8505.

## How was this patch tested?

Existing tests.

Author: Nick Pentreath <[email protected]>

Closes #13641 from MLnick/add-since-annotations.

(cherry picked from commit 37494a1)
Signed-off-by: Xiangrui Meng <[email protected]>
@MLnick MLnick deleted the add-since-annotations branch June 21, 2016 08:00
@MLnick
Copy link
Contributor Author

MLnick commented Jun 22, 2016

@mengxr I thought more about the UnaryTransformer cases, and I actually don't think it's necessary to bump the annotations for the classes, just the public methods/vals exposing Vector.

I thought the concrete classes would be binary incompatible but I don't actually think that is the case - and I just tested this (master against commit immediately prior to e2efe05) and apart from scalingVec it is fine. Also gels with reviewing MiMa changes in #12627. Sorry about that!

I've created SPARK-16127 to track further annotations that need updating related to new ml.linalg classes, and I think I will revert this particular change for UnaryTransformer classes in #13840.

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.

4 participants