OK. Data rules.
If this leads to error, remove it. I reverse my opinion On Thu, May 23, 2013 at 7:56 PM, Jake Mannix <[email protected]> wrote: > I dunno man, I think we've found 2 errors so far because the dev glibly did > the simplest possible thing and did for (Element e : vector), and they > really meant for (Element e : vector.nonZeroes()). > > My thinking is this: if you only have one kind of iterator, then it clearly > makes sense to implement Iterable. If you provide two different iterators, > neither of which is clearly the right one, then don't implement Iterable, > as it shows preference for one iterator. In this case, it actually shows > preference for the *wrong* iterator. > > > On Thu, May 23, 2013 at 7:33 PM, Ted Dunning <[email protected]> > wrote: > > > Looks good to me. > > > > I still don't like the all() usage. > > > > Can you leave the interface in place on the vector so one has the choice > of > > idioms? > > > > > > > > On Thu, May 23, 2013 at 7:06 PM, Jake Mannix <[email protected]> > > wrote: > > > > > It's done, patch passes tests, but it involves adding 308 lines > > > (removing 556 others!) across 84 classes in the codebase, so it > probably > > > touches something you wrote. > > > > > > So I'm going to let this diff marinade for a day or so, but if you feel > > > like you've got some thoughts (+1's? -1's? I'll even take some > > +epsilons > > > if you're feeling cowardly) on it, feel free to comment on the > > > ticket<https://issues.apache.org/jira/browse/MAHOUT-1227> or > > > on the review <https://reviews.apache.org/r/11359> itself. > > > > > > I won't let the patch stick around forever though, as most every commit > > > that hits trunk will force me to regenerate this patch again, given its > > > breadth. > > > > > > -- > > > > > > -jake > > > > > > > > > -- > > -jake >
