Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/12413#discussion_r59979986
--- Diff:
sql/core/src/main/scala/org/apache/spark/sql/execution/stat/StatFunctions.scala
---
@@ -431,7 +431,7 @@ private[sql] object StatFunctions extends Logging {
s"exceed 1e4. Currently $columnSize")
val table = counts.groupBy(_.get(0)).map { case (col1Item, rows) =>
val countsRow = new GenericMutableRow(columnSize + 1)
- rows.foreach { (row: Row) =>
+ rows.foreach { row =>
--- End diff --
This seems like a tiny point but is actually a decent opportunity to talk
about how we collectively should write the most readable code. As I say, either
you mean all the types in this method are obvious to you (they aren't to me!),
or you mean you'd actually write just about every type in this method, and I
dare say no Scala programmer would. Putting aside cases where the type is
essential (methods, or where the inferred reference type must be overridden),
this is why I'd say the standard is certainly readability, not strictly whether
the type is obvious. I bet we mean the same thing. It's going to be a judgment
call that deserves relatively wide latitude, but that _does_ argue against
changing a call in the first place unless it seems materially better, and that
standard was not met on this line, I agree in retrospect. While I might undo
this, not some others you're pointing out, if you feel strongly about reverting
a few of these types of changes and that lets us close this ou
t, that LGTM
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
