[BEAM-10235] Adding a CountElms transform to the Go SDK. #11986
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
R: @lostluck |
lostluck
reviewed
Jun 11, 2020
|
|
||
| // 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. |
There was a problem hiding this 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.
There was a problem hiding this comment.
Done for Count. And for CountElms I removed the line because T's encoding doesn't need to be deterministic since it's not used as a key.
lostluck
reviewed
Jun 11, 2020
| return Sum(s, pre) | ||
| } | ||
|
|
||
| func mapFn(_ beam.T) int { |
There was a problem hiding this comment.
Sounds good to me. Changed this to countFn and the other to keyedCountFn.
lostluck
approved these changes
Jun 11, 2020
This transform counts the number of elements in a PTransform.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This transform counts the number of elements in a PTransform.
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
R: @username).[BEAM-XXX] Fixes bug in ApproximateQuantiles, where you replaceBEAM-XXXwith the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.CHANGES.mdwith noteworthy changes.See the Contributor Guide for more tips on how to make review process smoother.
Post-Commit Tests Status (on master branch)
Pre-Commit Tests Status (on master branch)
See .test-infra/jenkins/README for trigger phrase, status and link of all Jenkins jobs.