You are viewing a plain text version of this content. The canonical link for it is here.

Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2022/10/05 22:31:03 UTC

[GitHub] [beam] TheNeuralBit opened a new issue, #23511: [Feature Request]: Make CombineFns used in Python Combiners part of the public API

TheNeuralBit opened a new issue, #23511:
URL: https://github.com/apache/beam/issues/23511

   ### What would you like to happen?
   
   Currently the CombineFn implementations in combiners.py are [not included in `__all__`](https://github.com/apache/beam/blob/7980cb9a35cdb37ffd57a43ec9a1bfa5e204f5d7/sdks/python/apache_beam/transforms/combiners.py#L49) which makes them look like implementation details. I think we should expose these in a way that makes it clear they can be used directly (e.g. in GroupBy with aggregate_fields). Two possibilities:
   - Just add the CombineFn implementations to `__all__`
   - Add a `combineFn` method on the corresponding combiners - this would be analogous to the Java SDK. see [`Count.combineFn`](https://beam.apache.org/releases/javadoc/current/org/apache/beam/sdk/transforms/Count.html#combineFn--).
   
   ### Issue Priority
   
   Priority: 2
   
   ### Issue Component
   
   Component: sdk-py-core


-- 
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.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] tvalentyn commented on issue #23511: [Feature Request]: Make CombineFns used in Python Combiners part of the public API

Posted by GitBox <gi...@apache.org>.

tvalentyn commented on issue #23511:
URL: https://github.com/apache/beam/issues/23511#issuecomment-1269118603

   I think there is no concern adding public CombineFn implementations to `__all__`. Re second approach - there may be some ambiguity here for composite combiners. For example: would there be an expectation for defining "Top.SmallestPerKey.combineFn" or just Top.combineFn ?


-- 
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 closed issue #23511: [Feature Request]: Make CombineFns used in Python Combiners part of the public API

Posted by GitBox <gi...@apache.org>.

tvalentyn closed issue #23511: [Feature Request]: Make CombineFns used in Python Combiners part of the public API
URL: https://github.com/apache/beam/issues/23511


-- 
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