lostluck commented on a change in pull request #11986:
URL: https://github.com/apache/beam/pull/11986#discussion_r438868945
##########
File path: sdks/go/pkg/beam/transforms/stats/count.go
##########
@@ -18,18 +18,36 @@ package stats
import (
"github.com/apache/beam/sdks/go/pkg/beam"
+ "github.com/apache/beam/sdks/go/pkg/beam/core/typex"
)
-// Count counts the number of elements in a collection. It expects a
-// PCollection<T> as input and returns a PCollection<KV<T,int>>. T's encoding
-// must be a well-defined injection.
+// Count counts the number of appearances of each element in a collection. It
+// expects a PCollection<T> as input and returns a PCollection<KV<T,int>>. T's
+// encoding must be a well-defined injection.
func Count(s beam.Scope, col beam.PCollection) beam.PCollection {
s = s.Scope("stats.Count")
- pre := beam.ParDo(s, mapFn, col)
+ pre := beam.ParDo(s, keyedMapFn, col)
return SumPerKey(s, pre)
}
-func mapFn(elm beam.T) (beam.T, int) {
+func keyedMapFn(elm beam.T) (beam.T, int) {
return elm, 1
}
+
+// CountElms counts the number of elements in a collection. It expects a
+// PCollection<T> as input and returns a PCollection<int> of one element
+// containing the count. T's encoding must be a well-defined injection.
Review comment:
Consider removing that last sentence about injections (here and above).
Or we can change it!
I think it's supposed to require that the encodings be deterministic (which
is required since Count uses elements as keys) which is clearer than the formal
wording that currently exists.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
