I don't think there's a noticeable performance hit by the use of reverse in those cases. It was a quick set of changes and it helped understand what you look for. I didn't intend to nitpick, so I'll leave as is. I could have used a scala.Ordering implicitly/explicitly also, but seems overkill and don't want to necessarily start a discussion about what's best--unless one of the admins deems this important.
I'll only keep the use of take and tail over using slice and switch over to math.min where indicated. This after I follow Henry's timely advice--thanks, Henry. cheers. On Thu, Apr 10, 2014 at 2:10 PM, Reynold Xin <r...@databricks.com> wrote: > Thanks for contributing! > > I think often unless the feature is gigantic, you can send a pull request > directly for discussion. One rule of thumb in the Spark code base is that > we typically prefer readability over conciseness, and thus we tend to avoid > using too much Scala magic or operator overloading. > > In this specific case, do you know if using - instead of reverse improve > performance? I personally find it slightly awkward to use underscore right > after negation ... > > > The tail change looks good to me. > > For foldLeft, I agree with you that the old way is more readable (although > less idiomatic scala). > > > > > On Thu, Apr 10, 2014 at 1:48 PM, Ignacio Zendejas < > ignacio.zendejas...@gmail.com> wrote: > > > Hi, all - > > > > First off, I want to say that I love spark and am very excited about > > MLBase. I'd love to contribute now that I have some time, but before I do > > that I'd like to familiarize myself with the process. > > > > In looking for a few projects and settling on one which I'll discuss in > > another thread, I found some very minor optimizations I could contribute, > > again, as part of this first step. > > > > Before I initiate a PR, I've gone ahead and tested style, ran tests, etc > > per the instructions, but I'd still like to have someone quickly glance > > over it and ensure that these are JIRA worthy. > > > > Commit: > > > > > https://github.com/izendejas/spark/commit/81065aed9987c1b08cd5784b7a6153e26f3f7402 > > > > To summarize: > > > > * I got rid of some SeqLike.reverse calls when sorting by descending > order > > * replaced slice(1, length) calls with the much safer (avoids IOOBEs) and > > more readable .tail calls > > * used a foldleft to avoid using mutable variables in NaiveBayes code > > > > This last one is meant to understand what's valued more between idiomatic > > Scala development or readability. I'm personally a fan of foldLefts where > > applicable, but do think they're a bit less readable. > > > > Thanks, > > Ignacio > > >
