You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by holdenk <gi...@git.apache.org> on 2018/06/13 16:44:08 UTC

[GitHub] spark pull request #21321: [SPARK-24268][SQL] Use datatype.simpleString in e...

Github user holdenk commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21321#discussion_r195156323
  
    --- Diff: mllib/src/main/scala/org/apache/spark/ml/feature/FeatureHasher.scala ---
    @@ -208,7 +208,8 @@ class FeatureHasher(@Since("2.3.0") override val uid: String) extends Transforme
           require(dataType.isInstanceOf[NumericType] ||
             dataType.isInstanceOf[StringType] ||
             dataType.isInstanceOf[BooleanType],
    -        s"FeatureHasher requires columns to be of NumericType, BooleanType or StringType. " +
    +        s"FeatureHasher requires columns to be of ${NumericType.simpleString}, " +
    --- End diff --
    
    In the original PR it doesn't seem like we rewrote the constant types only the dynamic ones (and this PR also doesn't seem to consistently rewrite the constant types referenced). What's the reason/how do you decide which ones you want to rewrite to ref simpleString?


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org