incubator-crunch-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Josh Wills <jwi...@cloudera.com>
Subject Re: Review Request: Add PGroupedTable.combineValues(Aggregator) and Aggregators.
Date Sun, 11 Nov 2012 21:12:12 GMT
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 <matt@mafr.de> 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>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message