I'm +1 for Aggregator as a top-level interface, we should make it easier to find.
On Sun, Nov 11, 2012 at 6:10 AM, Matthias Friedrich <[email protected]> wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/7858/ > ----------------------------------------------------------- > > (Updated Nov. 11, 2012, 2:10 p.m.) > > > Review request for crunch. > > > Changes > ------- > > Updated based on Josh's review, deprecated most of CombineFn and added > Javadoc to Aggregators. As far as I'm concerned, this is pretty much ready > to go. > > One open issue: Have a look at > http://tmp.mafr.de/crunch/apidocs/0.5.0/org/apache/crunch/fn/Aggregators.html- > Due to a bug in the javadoc tool (open since 2006) and the way it > displays inner classes, we get "CombineFn.Aggregator" everywhere. Should we > make Aggregator a top-level type? This would clean up documentation quite a > bit. > > > Description > ------- > > Here's a patch to test ReviewBoard and to give you an impression of the > new Aggregators class and the changes this would require. I have simplified > things even further than I originally planned, so I need some feedback if > I'm on the right track here. > > Even when approved, I will not commit this immediately. There are a few > things that need to be done that I'll discuss on crunch-dev. > > > Diffs (updated) > ----- > > > crunch-contrib/src/main/java/org/apache/crunch/contrib/bloomfilter/BloomFilterFactory.java > 825b445 > > crunch-examples/src/main/java/org/apache/crunch/examples/AverageBytesByIP.java > 8abbb73 > > crunch-examples/src/main/java/org/apache/crunch/examples/TotalBytesByIP.java > 44776ea > > crunch-examples/src/main/java/org/apache/crunch/examples/WordAggregationHBase.java > 691721d > crunch/src/it/java/org/apache/crunch/CollectionsIT.java 0d5803e > crunch/src/it/java/org/apache/crunch/WordCountIT.java 5124663 > crunch/src/it/java/org/apache/crunch/fn/AggregatorsIT.java PRE-CREATION > crunch/src/it/java/org/apache/crunch/lib/CogroupIT.java b6f5029 > crunch/src/it/java/org/apache/crunch/test/Tests.java PRE-CREATION > crunch/src/it/resources/org/apache/crunch/fn/AggregatorsITData/ints.txt > PRE-CREATION > crunch/src/main/java/org/apache/crunch/CombineFn.java d45940b > crunch/src/main/java/org/apache/crunch/PGroupedTable.java e727b70 > crunch/src/main/java/org/apache/crunch/fn/Aggregators.java PRE-CREATION > > crunch/src/main/java/org/apache/crunch/impl/mem/collect/MemGroupedTable.java > 0ee4c3f > > crunch/src/main/java/org/apache/crunch/impl/mr/collect/PGroupedTableImpl.java > fee381d > crunch/src/main/java/org/apache/crunch/lib/Aggregate.java f28cca4 > crunch/src/test/java/org/apache/crunch/fn/AggregatorsTest.java > PRE-CREATION > pom.xml 84d481f > > Diff: https://reviews.apache.org/r/7858/diff/ > > > Testing > ------- > > Integration test works, as did a simple example on a single node cluster. > Added an integration test and unit tests based on CombineFnTest. > > > Thanks, > > Matthias Friedrich > > -- Director of Data Science Cloudera <http://www.cloudera.com> Twitter: @josh_wills <http://twitter.com/josh_wills>
