mahout-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Pat Ferrel <...@occamsmachete.com>
Subject Re: Code quality questions
Date Sat, 24 Jan 2015 18:23:55 GMT
BTW #1 _is_ used for operators in Mahout but seldom for named functions from what I’ve seen

On Jan 24, 2015, at 9:54 AM, Pat Ferrel <pat@occamsmachete.com> wrote:

The comments are formatted using IDE defaults, I agree they can be cleaned up.

The Scala docs here: http://docs.scala-lang.org/style/method-invocation.html are the usual
source of guidance. There have been a couple exceptions to these guidelines including:
1) We (in much of the mahout scala code I’ve seen) don’t seem to follow the "no dot”
infix style recommended in the guide http://docs.scala-lang.org/style/method-invocation.html
There was a discussion of this some time ago and seemed be consensus for a more java-like
form.

As to removing org.apache.mahout.common.Pair this seems like a good idea in light of the recent
refactoring. I took it from a quick back of the envelope function that Sebastian wrote over
a year ago when it was a non-issue. I’ll look at that the next time I’m in the code. I
assume this would remove the need for Pair being in the module replacing mr-leagacy for Scala?

On Jan 24, 2015, at 5:10 AM, Gokhan Capan <gkhncpn@gmail.com> wrote:

+1 for favoring native scala types.

I think in terms of Scala code, we need a clear style standards definition
to adhere to.


Gokhan

On Fri, Jan 23, 2015 at 9:38 PM, Dmitriy Lyubimov <dlieu.7@gmail.com> wrote:

> in TextDelimitedReaderWriter.scala:
> 
> ===========================
> val itemList:
> collection.mutable.MutableList[org.apache.mahout.common.Pair[Integer,
> Double]] = new
> collection.mutable.MutableList[org.apache.mahout.common.Pair[Integer,
> Double]]
>       for (ve <- itemVector.nonZeroes) {
>         val item: org.apache.mahout.common.Pair[Integer, Double] = new
> org.apache.mahout.common.Pair[Integer, Double](ve.index, ve.get)
>         itemList += item
>       }
> ================================
> 
> (1) why scala code attempts to use commons.pair? What was wrong about
> native Tuple type of scala? (I am trying to clean out mrlegacy dependencies
> from spark module).
> 
> (2) why it is so horribly styled (even for me)? comments are misaligned,
> the lines routinely exceed 120 characters?
> 
> Can these problems please be addressed? in particular, stuff like
> o.a.m.common.Pair? And why it is even signed off on in the first place by
> committers despite of clear style violations?
> 
> thank you.
> 



Mime
View raw message