You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by "svetakvsundhar (via GitHub)" <gi...@apache.org> on 2023/04/24 19:37:19 UTC
[GitHub] [beam] svetakvsundhar opened a new pull request, #26406: Updating Combiners.Top PTransform Documentation
svetakvsundhar opened a new pull request, #26406:
URL: https://github.com/apache/beam/pull/26406
Fixes #19819.
------------------------
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
- [ ] Mention the appropriate issue in your description (for example: `addresses #123`), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, comment `fixes #<ISSUE NUMBER>` instead.
- [ ] Update `CHANGES.md` with noteworthy changes.
- [ ] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
See the [Contributor Guide](https://beam.apache.org/contribute) for more tips on [how to make review process smoother](https://beam.apache.org/contribute/get-started-contributing/#make-the-reviewers-job-easier).
To check the build health, please visit [https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md](https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md)
GitHub Actions Tests Status (on master branch)
------------------------------------------------------------------------------------------------
[](https://github.com/apache/beam/actions?query=workflow%3A%22Build+python+source+distribution+and+wheels%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Python+Tests%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Java+Tests%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Go+tests%22+branch%3Amaster+event%3Aschedule)
See [CI.md](https://github.com/apache/beam/blob/master/CI.md) for more information about GitHub Actions CI.
--
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.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] tvalentyn commented on a diff in pull request #26406: Updating Combiners.Top PTransform Documentation
Posted by "tvalentyn (via GitHub)" <gi...@apache.org>.
tvalentyn commented on code in PR #26406:
URL: https://github.com/apache/beam/pull/26406#discussion_r1177941132
##########
sdks/python/apache_beam/transforms/combiners.py:
##########
@@ -195,11 +195,12 @@ class Top(object):
@with_input_types(T)
@with_output_types(List[T])
class Of(CombinerWithoutDefaults):
- """Obtain a list of the compare-most N elements in a PCollection.
+ """Obtain a list of the n greatest elements in a PCollection.
Review Comment:
```suggestion
"""Obtain a list of the N greatest elements in a PCollection.
```
##########
sdks/python/apache_beam/transforms/combiners.py:
##########
@@ -195,11 +195,12 @@ class Top(object):
@with_input_types(T)
@with_output_types(List[T])
class Of(CombinerWithoutDefaults):
- """Obtain a list of the compare-most N elements in a PCollection.
+ """Obtain a list of the n greatest elements in a PCollection.
This transform will retrieve the n greatest elements in the PCollection
- to which it is applied, where "greatest" is determined by the comparator
- function supplied as the compare argument.
+ to which it is applied, where "greatest" is determined by a
+ function supplied as the key or reverse arguments.
+
Review Comment:
nit: extra newline
##########
sdks/python/apache_beam/transforms/combiners.py:
##########
@@ -249,12 +250,13 @@ def expand(self, pcoll):
@with_input_types(Tuple[K, V])
@with_output_types(Tuple[K, List[V]])
class PerKey(ptransform.PTransform):
- """Identifies the compare-most N elements associated with each key.
+ """Identifies the N greatest elements associated with each key.
This transform will produce a PCollection mapping unique keys in the input
PCollection to the n greatest elements with which they are associated, where
- "greatest" is determined by the comparator function supplied as the compare
- argument in the initializer.
+ "greatest" is determined by a function supplied as the key or
Review Comment:
let's back-tick param names. eg. \`key\`
##########
sdks/python/apache_beam/transforms/combiners.py:
##########
@@ -249,12 +250,13 @@ def expand(self, pcoll):
@with_input_types(Tuple[K, V])
@with_output_types(Tuple[K, List[V]])
class PerKey(ptransform.PTransform):
- """Identifies the compare-most N elements associated with each key.
+ """Identifies the N greatest elements associated with each key.
This transform will produce a PCollection mapping unique keys in the input
PCollection to the n greatest elements with which they are associated, where
- "greatest" is determined by the comparator function supplied as the compare
- argument in the initializer.
+ "greatest" is determined by a function supplied as the key or
+ reverse arguments.
+
Review Comment:
nit: extra newline
--
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.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] tvalentyn commented on pull request #26406: Updating Combiners.Top PTransform Documentation
Posted by "tvalentyn (via GitHub)" <gi...@apache.org>.
tvalentyn commented on PR #26406:
URL: https://github.com/apache/beam/pull/26406#issuecomment-1525803052
manually checked that docs, lint and formatter precommits passed.
--
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.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] codecov[bot] commented on pull request #26406: Updating Combiners.Top PTransform Documentation
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #26406:
URL: https://github.com/apache/beam/pull/26406#issuecomment-1520743364
## [Codecov](https://codecov.io/gh/apache/beam/pull/26406?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#26406](https://codecov.io/gh/apache/beam/pull/26406?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (7fd10aa) into [master](https://codecov.io/gh/apache/beam/commit/44a17cbe429699160e689bd88935c32c6c2de6b2?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (44a17cb) will **not change** coverage.
> The diff coverage is `n/a`.
```diff
@@ Coverage Diff @@
## master #26406 +/- ##
=======================================
Coverage 81.07% 81.07%
=======================================
Files 469 469
Lines 67199 67199
=======================================
Hits 54483 54483
Misses 12716 12716
```
| [Impacted Files](https://codecov.io/gh/apache/beam/pull/26406?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [sdks/python/apache\_beam/transforms/combiners.py](https://codecov.io/gh/apache/beam/pull/26406?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy9jb21iaW5lcnMucHk=) | `93.43% <ø> (ø)` | |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
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.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] tvalentyn merged pull request #26406: Updating Combiners.Top PTransform Documentation
Posted by "tvalentyn (via GitHub)" <gi...@apache.org>.
tvalentyn merged PR #26406:
URL: https://github.com/apache/beam/pull/26406
--
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.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] github-actions[bot] commented on pull request #26406: Updating Combiners.Top PTransform Documentation
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #26406:
URL: https://github.com/apache/beam/pull/26406#issuecomment-1520754986
Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control
--
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.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] svetakvsundhar commented on pull request #26406: Updating Combiners.Top PTransform Documentation
Posted by "svetakvsundhar (via GitHub)" <gi...@apache.org>.
svetakvsundhar commented on PR #26406:
URL: https://github.com/apache/beam/pull/26406#issuecomment-1520826038
Run Python_Runners PreCommit
--
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.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] tvalentyn commented on pull request #26406: Updating Combiners.Top PTransform Documentation
Posted by "tvalentyn (via GitHub)" <gi...@apache.org>.
tvalentyn commented on PR #26406:
URL: https://github.com/apache/beam/pull/26406#issuecomment-1521582261
per https://github.com/apache/beam/issues/19819#issuecomment-1367446140 we should remove references to `compare` instead of adding new ones.
also, Beam is Py3 only now.
--
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.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] svetakvsundhar commented on pull request #26406: Updating Combiners.Top PTransform Documentation
Posted by "svetakvsundhar (via GitHub)" <gi...@apache.org>.
svetakvsundhar commented on PR #26406:
URL: https://github.com/apache/beam/pull/26406#issuecomment-1520753285
r: @tvalentyn
--
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.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org