{"action":"created","comment":{"url":"https://api.github.com/repos/CommBank/coppersmith/pulls/comments/65013815","id":65013815,"diff_hunk":"@@ -112,31 +120,99 @@ trait AggregationFeatureSet[S] extends FeatureSet[(EntityId, Iterable[S])] {\n \n   // These allow aggregators to be created without specifying type args that\n   // would otherwise be required if calling the delegated methods directly\n-  def size: Aggregator[S, Long, Long]                                   = Aggregator.size\n-  def count(where: S => Boolean = _ => true): Aggregator[S, Long, Long] = Aggregator.count(where)\n-  def avg[V](v: S => Double): Aggregator[S, AveragedValue, Double]      = AggregationFeature.avg[S](v)\n-  def max[V : Ordering](v: S => V): Aggregator[S, V, V]                 = AggregationFeature.max[S, V](v)\n-  def min[V : Ordering](v: S => V): Aggregator[S, V, V]                 = AggregationFeature.min[S, V](v)\n-  def maxBy[O : Ordering, V](o: S => O)(v: S => V): Aggregator[S, S, V] = AggregationFeature.maxBy[S, O, V](o)(v)\n-  def minBy[O : Ordering, V](o: S => O)(v: S => V): Aggregator[S, S, V] = AggregationFeature.minBy[S, O, V](o)(v)\n-  def sum[V : Monoid]  (v: S => V): Aggregator[S, V, V]                 = Aggregator.prepareMonoid(v)\n-  def uniqueCountBy[T](f : S => T): Aggregator[S, Set[T], Int]          = AggregationFeature.uniqueCountBy(f)\n+  def size: Aggregator[S, Long, Long] =\n+    Aggregator.size\n+  def count(where: S => Boolean = _ => true): Aggregator[S, Long, Long] =\n+    Aggregator.count(where)\n+  def avg[V](v: S => Double): Aggregator[S, AveragedValue, Double] =\n+    AggregationFeature.avg[S](v)\n+  def avgBigDec[V](v: S => BigDecimal): Aggregator[S, BigDAveragedValue, BigDecimal] =\n+    AggregationFeature.avgBigDec[S](v)\n+  def max[V : Ordering](v: S => V): Aggregator[S, V, V] =\n+    AggregationFeature.max[S, V](v)\n+  def min[V : Ordering](v: S => V): Aggregator[S, V, V] =\n+    AggregationFeature.min[S, V](v)\n+  def maxBy[O : Ordering, V](o: S => O)(v: S => V): Aggregator[S, S, V] =\n+    AggregationFeature.maxBy[S, O, V](o)(v)\n+  def minBy[O : Ordering, V](o: S => O)(v: S => V): Aggregator[S, S, V] =\n+    AggregationFeature.minBy[S, O, V](o)(v)\n+  def sum[V : Monoid](v: S => V): Aggregator[S, V, V] =\n+    Aggregator.prepareMonoid(v)\n+  def uniqueCountBy[T](f : S => T): Aggregator[S, Set[T], Int] =\n+    AggregationFeature.uniqueCountBy(f)\n }\n \n object AggregationFeature {\n-  def avg[T](t: T => Double): Aggregator[T, AveragedValue, Double]         =\n+  import BigDAverages._\n+\n+  def avg[T](t: T => Double): Aggregator[T, AveragedValue, Double] =\n     AveragedValue.aggregator.composePrepare[T](t)\n+  def avgBigDec[T](t: T => BigDecimal): Aggregator[T, BigDAveragedValue, BigDecimal] =\n+    BigDAverager.composePrepare[T](t)\n   def maxBy[T, O : Ordering, V](o: T => O)(v: T => V): Aggregator[T, T, V] =\n     Aggregator.maxBy[T, O](o).andThenPresent[V](v)\n   def minBy[T, O : Ordering, V](o: T => O)(v: T => V): Aggregator[T, T, V] =\n     Aggregator.minBy[T, O](o).andThenPresent[V](v)\n \n-  def max[T, V : Ordering](v: T => V): Aggregator[T, V, V]                 = Aggregator.max[V].composePrepare[T](v)\n-  def min[T, V : Ordering](v: T => V): Aggregator[T, V, V]                 = Aggregator.min[V].composePrepare[T](v)\n-  def uniqueCountBy[S, T](f : S => T): Aggregator[S, Set[T], Int]          = Aggregator.uniqueCount[T].composePrepare(f)\n+  def max[T, V : Ordering](v: T => V): Aggregator[T, V, V]        = Aggregator.max[V].composePrepare[T](v)\n+  def min[T, V : Ordering](v: T => V): Aggregator[T, V, V]        = Aggregator.min[V].composePrepare[T](v)\n+  def uniqueCountBy[S, T](f : S => T): Aggregator[S, Set[T], Int] = Aggregator.uniqueCount[T].composePrepare(f)\n \n   // TODO: Would be surprised if this doesn't exist elsewhere\n   implicit class AlgebirdSemigroup[T](s: Semigroup[T]) {\n     def toScalaz = new scalaz.Semigroup[T] { def append(t1: T, t2: =>T): T = s.plus(t1, t2) }\n   }\n+\n+  // Duplicate code from com.twitter.algebird to allow BigDecimals\n+\n+  object BigDAverages {\n+    import com.twitter.algebird.{Group, MonoidAggregator}\n+\n+    object BigDAveragedValue {\n+      implicit val group = BigDAveragedGroup\n+      def numericAggregator[N](implicit num: Numeric[N]): MonoidAggregator[N, BigDAveragedValue, BigDecimal] =\n+        Aggregator.prepareMonoid { n: N => new BigDAveragedValue(1L, num.toDouble(n)) }\n+          .andThenPresent(_.value)\n+    }\n+\n+    case class BigDAveragedValue(count: Long, value: BigDecimal)\n+\n+    object BigDAverager extends MonoidAggregator[BigDecimal, BigDAveragedValue, BigDecimal] {\n+      val monoid = BigDAveragedGroup\n+\n+      def prepare(value: BigDecimal) = new BigDAveragedValue(1L, value)\n+\n+      def present(average: BigDAveragedValue) = average.value\n+    }\n+\n+    object BigDAveragedGroup extends Group[BigDAveragedValue] {\n+      val zero = BigDAveragedValue(0L, 0.0)\n+\n+      override def isNonZero(av: BigDAveragedValue) = (av.count != 0L)\n+\n+      override def negate(av: BigDAveragedValue) = BigDAveragedValue(-av.count, av.value)\n+\n+      def plus(cntAve1: BigDAveragedValue, cntAve2: BigDAveragedValue): BigDAveragedValue = {\n+        val (big, small) = if (cntAve1.count >= cntAve2.count)\n+          (cntAve1, cntAve2)\n+        else\n+          (cntAve2, cntAve1)\n+        val n = big.count\n+        val k = small.count\n+        val newCnt = n + k\n+        if (newCnt == n) {\n+          // Handle zero without allocation\n+          big\n+        } else if (newCnt == 0L) {\n+          zero\n+        } else {\n+          val an = big.value\n+          val ak = small.value\n+          val newAve = (n * an + k * ak) / newCnt\n+          new BigDAveragedValue(newCnt, newAve)\n+        }\n+      }\n+    }\n+","path":"core/src/main/scala/commbank/coppersmith/FeatureSet.scala","position":126,"original_position":126,"commit_id":"158bb55d93ffc50ffbe500a3470d139c36552756","original_commit_id":"158bb55d93ffc50ffbe500a3470d139c36552756","user":{"login":"dkristian","id":551256,"avatar_url":"https://avatars.githubusercontent.com/u/551256?v=3","gravatar_id":"","url":"https://api.github.com/users/dkristian","html_url":"https://github.com/dkristian","followers_url":"https://api.github.com/users/dkristian/followers","following_url":"https://api.github.com/users/dkristian/following{/other_user}","gists_url":"https://api.github.com/users/dkristian/gists{/gist_id}","starred_url":"https://api.github.com/users/dkristian/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dkristian/subscriptions","organizations_url":"https://api.github.com/users/dkristian/orgs","repos_url":"https://api.github.com/users/dkristian/repos","events_url":"https://api.github.com/users/dkristian/events{/privacy}","received_events_url":"https://api.github.com/users/dkristian/received_events","type":"User","site_admin":false},"body":"Minor: additional vertical space","created_at":"2016-05-29T23:26:29Z","updated_at":"2016-05-29T23:26:29Z","html_url":"https://github.com/CommBank/coppersmith/pull/30#discussion_r65013815","pull_request_url":"https://api.github.com/repos/CommBank/coppersmith/pulls/30","_links":{"self":{"href":"https://api.github.com/repos/CommBank/coppersmith/pulls/comments/65013815"},"html":{"href":"https://github.com/CommBank/coppersmith/pull/30#discussion_r65013815"},"pull_request":{"href":"https://api.github.com/repos/CommBank/coppersmith/pulls/30"}}},"pull_request":{"url":"https://api.github.com/repos/CommBank/coppersmith/pulls/30","id":71484877,"html_url":"https://github.com/CommBank/coppersmith/pull/30","diff_url":"https://github.com/CommBank/coppersmith/pull/30.diff","patch_url":"https://github.com/CommBank/coppersmith/pull/30.patch","issue_url":"https://api.github.com/repos/CommBank/coppersmith/issues/30","number":30,"state":"open","locked":false,"title":"Use BigDecimal for Decimal feature value","user":{"login":"ewanv","id":4424599,"avatar_url":"https://avatars.githubusercontent.com/u/4424599?v=3","gravatar_id":"","url":"https://api.github.com/users/ewanv","html_url":"https://github.com/ewanv","followers_url":"https://api.github.com/users/ewanv/followers","following_url":"https://api.github.com/users/ewanv/following{/other_user}","gists_url":"https://api.github.com/users/ewanv/gists{/gist_id}","starred_url":"https://api.github.com/users/ewanv/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ewanv/subscriptions","organizations_url":"https://api.github.com/users/ewanv/orgs","repos_url":"https://api.github.com/users/ewanv/repos","events_url":"https://api.github.com/users/ewanv/events{/privacy}","received_events_url":"https://api.github.com/users/ewanv/received_events","type":"User","site_admin":false},"body":"Creates `Feature.Value.FloatingPoint` to hold `Double` values, and changes the value type of `Decimal` to `BigDecimal`. It may be better not to create a `FloatingPoint`, and allow `Decimal` to accept `Double` values, and convert them to `BigDecimal`. This change would then not break existing client code.\r\n\r\nThis PR also makes the `avg` aggregator use `BigDecimal` instead of `Double`.","created_at":"2016-05-26T00:41:00Z","updated_at":"2016-05-29T23:26:29Z","closed_at":null,"merged_at":null,"merge_commit_sha":"41d4e59abe57ed91df2e2b8cf050beed56af0bc7","assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/CommBank/coppersmith/pulls/30/commits","review_comments_url":"https://api.github.com/repos/CommBank/coppersmith/pulls/30/comments","review_comment_url":"https://api.github.com/repos/CommBank/coppersmith/pulls/comments{/number}","comments_url":"https://api.github.com/repos/CommBank/coppersmith/issues/30/comments","statuses_url":"https://api.github.com/repos/CommBank/coppersmith/statuses/158bb55d93ffc50ffbe500a3470d139c36552756","head":{"label":"CommBank:big-decimal","ref":"big-decimal","sha":"158bb55d93ffc50ffbe500a3470d139c36552756","user":{"login":"CommBank","id":4240076,"avatar_url":"https://avatars.githubusercontent.com/u/4240076?v=3","gravatar_id":"","url":"https://api.github.com/users/CommBank","html_url":"https://github.com/CommBank","followers_url":"https://api.github.com/users/CommBank/followers","following_url":"https://api.github.com/users/CommBank/following{/other_user}","gists_url":"https://api.github.com/users/CommBank/gists{/gist_id}","starred_url":"https://api.github.com/users/CommBank/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/CommBank/subscriptions","organizations_url":"https://api.github.com/users/CommBank/orgs","repos_url":"https://api.github.com/users/CommBank/repos","events_url":"https://api.github.com/users/CommBank/events{/privacy}","received_events_url":"https://api.github.com/users/CommBank/received_events","type":"Organization","site_admin":false},"repo":{"id":55942036,"name":"coppersmith","full_name":"CommBank/coppersmith","owner":{"login":"CommBank","id":4240076,"avatar_url":"https://avatars.githubusercontent.com/u/4240076?v=3","gravatar_id":"","url":"https://api.github.com/users/CommBank","html_url":"https://github.com/CommBank","followers_url":"https://api.github.com/users/CommBank/followers","following_url":"https://api.github.com/users/CommBank/following{/other_user}","gists_url":"https://api.github.com/users/CommBank/gists{/gist_id}","starred_url":"https://api.github.com/users/CommBank/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/CommBank/subscriptions","organizations_url":"https://api.github.com/users/CommBank/orgs","repos_url":"https://api.github.com/users/CommBank/repos","events_url":"https://api.github.com/users/CommBank/events{/privacy}","received_events_url":"https://api.github.com/users/CommBank/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/CommBank/coppersmith","description":"Feature Engineering as Composable Functions","fork":false,"url":"https://api.github.com/repos/CommBank/coppersmith","forks_url":"https://api.github.com/repos/CommBank/coppersmith/forks","keys_url":"https://api.github.com/repos/CommBank/coppersmith/keys{/key_id}","collaborators_url":"https://api.github.com/repos/CommBank/coppersmith/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/CommBank/coppersmith/teams","hooks_url":"https://api.github.com/repos/CommBank/coppersmith/hooks","issue_events_url":"https://api.github.com/repos/CommBank/coppersmith/issues/events{/number}","events_url":"https://api.github.com/repos/CommBank/coppersmith/events","assignees_url":"https://api.github.com/repos/CommBank/coppersmith/assignees{/user}","branches_url":"https://api.github.com/repos/CommBank/coppersmith/branches{/branch}","tags_url":"https://api.github.com/repos/CommBank/coppersmith/tags","blobs_url":"https://api.github.com/repos/CommBank/coppersmith/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/CommBank/coppersmith/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/CommBank/coppersmith/git/refs{/sha}","trees_url":"https://api.github.com/repos/CommBank/coppersmith/git/trees{/sha}","statuses_url":"https://api.github.com/repos/CommBank/coppersmith/statuses/{sha}","languages_url":"https://api.github.com/repos/CommBank/coppersmith/languages","stargazers_url":"https://api.github.com/repos/CommBank/coppersmith/stargazers","contributors_url":"https://api.github.com/repos/CommBank/coppersmith/contributors","subscribers_url":"https://api.github.com/repos/CommBank/coppersmith/subscribers","subscription_url":"https://api.github.com/repos/CommBank/coppersmith/subscription","commits_url":"https://api.github.com/repos/CommBank/coppersmith/commits{/sha}","git_commits_url":"https://api.github.com/repos/CommBank/coppersmith/git/commits{/sha}","comments_url":"https://api.github.com/repos/CommBank/coppersmith/comments{/number}","issue_comment_url":"https://api.github.com/repos/CommBank/coppersmith/issues/comments{/number}","contents_url":"https://api.github.com/repos/CommBank/coppersmith/contents/{+path}","compare_url":"https://api.github.com/repos/CommBank/coppersmith/compare/{base}...{head}","merges_url":"https://api.github.com/repos/CommBank/coppersmith/merges","archive_url":"https://api.github.com/repos/CommBank/coppersmith/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/CommBank/coppersmith/downloads","issues_url":"https://api.github.com/repos/CommBank/coppersmith/issues{/number}","pulls_url":"https://api.github.com/repos/CommBank/coppersmith/pulls{/number}","milestones_url":"https://api.github.com/repos/CommBank/coppersmith/milestones{/number}","notifications_url":"https://api.github.com/repos/CommBank/coppersmith/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/CommBank/coppersmith/labels{/name}","releases_url":"https://api.github.com/repos/CommBank/coppersmith/releases{/id}","deployments_url":"https://api.github.com/repos/CommBank/coppersmith/deployments","created_at":"2016-04-11T04:38:16Z","updated_at":"2016-05-17T12:47:53Z","pushed_at":"2016-05-27T07:00:27Z","git_url":"git://github.com/CommBank/coppersmith.git","ssh_url":"git@github.com:CommBank/coppersmith.git","clone_url":"https://github.com/CommBank/coppersmith.git","svn_url":"https://github.com/CommBank/coppersmith","homepage":null,"size":839,"stargazers_count":14,"watchers_count":14,"language":"Scala","has_issues":true,"has_downloads":true,"has_wiki":false,"has_pages":true,"forks_count":2,"mirror_url":null,"open_issues_count":2,"forks":2,"open_issues":2,"watchers":14,"default_branch":"master"}},"base":{"label":"CommBank:master","ref":"master","sha":"d0228689127fa5d6008df65c1f78c3fccc33293e","user":{"login":"CommBank","id":4240076,"avatar_url":"https://avatars.githubusercontent.com/u/4240076?v=3","gravatar_id":"","url":"https://api.github.com/users/CommBank","html_url":"https://github.com/CommBank","followers_url":"https://api.github.com/users/CommBank/followers","following_url":"https://api.github.com/users/CommBank/following{/other_user}","gists_url":"https://api.github.com/users/CommBank/gists{/gist_id}","starred_url":"https://api.github.com/users/CommBank/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/CommBank/subscriptions","organizations_url":"https://api.github.com/users/CommBank/orgs","repos_url":"https://api.github.com/users/CommBank/repos","events_url":"https://api.github.com/users/CommBank/events{/privacy}","received_events_url":"https://api.github.com/users/CommBank/received_events","type":"Organization","site_admin":false},"repo":{"id":55942036,"name":"coppersmith","full_name":"CommBank/coppersmith","owner":{"login":"CommBank","id":4240076,"avatar_url":"https://avatars.githubusercontent.com/u/4240076?v=3","gravatar_id":"","url":"https://api.github.com/users/CommBank","html_url":"https://github.com/CommBank","followers_url":"https://api.github.com/users/CommBank/followers","following_url":"https://api.github.com/users/CommBank/following{/other_user}","gists_url":"https://api.github.com/users/CommBank/gists{/gist_id}","starred_url":"https://api.github.com/users/CommBank/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/CommBank/subscriptions","organizations_url":"https://api.github.com/users/CommBank/orgs","repos_url":"https://api.github.com/users/CommBank/repos","events_url":"https://api.github.com/users/CommBank/events{/privacy}","received_events_url":"https://api.github.com/users/CommBank/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/CommBank/coppersmith","description":"Feature Engineering as Composable Functions","fork":false,"url":"https://api.github.com/repos/CommBank/coppersmith","forks_url":"https://api.github.com/repos/CommBank/coppersmith/forks","keys_url":"https://api.github.com/repos/CommBank/coppersmith/keys{/key_id}","collaborators_url":"https://api.github.com/repos/CommBank/coppersmith/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/CommBank/coppersmith/teams","hooks_url":"https://api.github.com/repos/CommBank/coppersmith/hooks","issue_events_url":"https://api.github.com/repos/CommBank/coppersmith/issues/events{/number}","events_url":"https://api.github.com/repos/CommBank/coppersmith/events","assignees_url":"https://api.github.com/repos/CommBank/coppersmith/assignees{/user}","branches_url":"https://api.github.com/repos/CommBank/coppersmith/branches{/branch}","tags_url":"https://api.github.com/repos/CommBank/coppersmith/tags","blobs_url":"https://api.github.com/repos/CommBank/coppersmith/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/CommBank/coppersmith/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/CommBank/coppersmith/git/refs{/sha}","trees_url":"https://api.github.com/repos/CommBank/coppersmith/git/trees{/sha}","statuses_url":"https://api.github.com/repos/CommBank/coppersmith/statuses/{sha}","languages_url":"https://api.github.com/repos/CommBank/coppersmith/languages","stargazers_url":"https://api.github.com/repos/CommBank/coppersmith/stargazers","contributors_url":"https://api.github.com/repos/CommBank/coppersmith/contributors","subscribers_url":"https://api.github.com/repos/CommBank/coppersmith/subscribers","subscription_url":"https://api.github.com/repos/CommBank/coppersmith/subscription","commits_url":"https://api.github.com/repos/CommBank/coppersmith/commits{/sha}","git_commits_url":"https://api.github.com/repos/CommBank/coppersmith/git/commits{/sha}","comments_url":"https://api.github.com/repos/CommBank/coppersmith/comments{/number}","issue_comment_url":"https://api.github.com/repos/CommBank/coppersmith/issues/comments{/number}","contents_url":"https://api.github.com/repos/CommBank/coppersmith/contents/{+path}","compare_url":"https://api.github.com/repos/CommBank/coppersmith/compare/{base}...{head}","merges_url":"https://api.github.com/repos/CommBank/coppersmith/merges","archive_url":"https://api.github.com/repos/CommBank/coppersmith/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/CommBank/coppersmith/downloads","issues_url":"https://api.github.com/repos/CommBank/coppersmith/issues{/number}","pulls_url":"https://api.github.com/repos/CommBank/coppersmith/pulls{/number}","milestones_url":"https://api.github.com/repos/CommBank/coppersmith/milestones{/number}","notifications_url":"https://api.github.com/repos/CommBank/coppersmith/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/CommBank/coppersmith/labels{/name}","releases_url":"https://api.github.com/repos/CommBank/coppersmith/releases{/id}","deployments_url":"https://api.github.com/repos/CommBank/coppersmith/deployments","created_at":"2016-04-11T04:38:16Z","updated_at":"2016-05-17T12:47:53Z","pushed_at":"2016-05-27T07:00:27Z","git_url":"git://github.com/CommBank/coppersmith.git","ssh_url":"git@github.com:CommBank/coppersmith.git","clone_url":"https://github.com/CommBank/coppersmith.git","svn_url":"https://github.com/CommBank/coppersmith","homepage":null,"size":839,"stargazers_count":14,"watchers_count":14,"language":"Scala","has_issues":true,"has_downloads":true,"has_wiki":false,"has_pages":true,"forks_count":2,"mirror_url":null,"open_issues_count":2,"forks":2,"open_issues":2,"watchers":14,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/CommBank/coppersmith/pulls/30"},"html":{"href":"https://github.com/CommBank/coppersmith/pull/30"},"issue":{"href":"https://api.github.com/repos/CommBank/coppersmith/issues/30"},"comments":{"href":"https://api.github.com/repos/CommBank/coppersmith/issues/30/comments"},"review_comments":{"href":"https://api.github.com/repos/CommBank/coppersmith/pulls/30/comments"},"review_comment":{"href":"https://api.github.com/repos/CommBank/coppersmith/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/CommBank/coppersmith/pulls/30/commits"},"statuses":{"href":"https://api.github.com/repos/CommBank/coppersmith/statuses/158bb55d93ffc50ffbe500a3470d139c36552756"}}}}
{"actor":{"display_login":"dkristian"}}