Github user srowen commented on a diff in the pull request: https://github.com/apache/spark/pull/20367#discussion_r163911373 --- Diff: mllib/src/main/scala/org/apache/spark/ml/feature/CountVectorizer.scala --- @@ -160,6 +187,11 @@ class CountVectorizer @Since("1.5.0") (@Since("1.5.0") override val uid: String) } else { $(minDF) * input.cache().count() } + val maxDf = if ($(maxDF) >= 1.0) { + $(maxDF) + } else { + $(maxDF) * input.cache().count() + } --- End diff -- I wonder if it's best to check that the max is >= the min? this may be the only place that can be done as one might be a fraction and the other not. Also this ends up counting the input twice when both params are set, which might be worth avoiding. It could be computed and saved only when needed. (I suppose this also fails to unpersist `input`, which could happen right after `wordCounts.count()`. That's a slightly different issue, so not too concerned about whether to fix it here or not.)
--- --------------------------------------------------------------------- To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org