You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by "hjtran (via GitHub)" <gi...@apache.org> on 2025/07/31 17:28:06 UTC

[PR] implement main pcollection as default to dooutputstuple [beam]

hjtran opened a new pull request, #35754:
URL: https://github.com/apache/beam/pull/35754

   **Please** add a meaningful description for your change here
   
   ------------------------
   
   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://github.com/apache/beam/blob/master/CONTRIBUTING.md#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)
   ------------------------------------------------------------------------------------------------
   [![Build python source distribution and wheels](https://github.com/apache/beam/actions/workflows/build_wheels.yml/badge.svg?event=schedule&&?branch=master)](https://github.com/apache/beam/actions?query=workflow%3A%22Build+python+source+distribution+and+wheels%22+branch%3Amaster+event%3Aschedule)
   [![Python tests](https://github.com/apache/beam/actions/workflows/python_tests.yml/badge.svg?event=schedule&&?branch=master)](https://github.com/apache/beam/actions?query=workflow%3A%22Python+Tests%22+branch%3Amaster+event%3Aschedule)
   [![Java tests](https://github.com/apache/beam/actions/workflows/java_tests.yml/badge.svg?event=schedule&&?branch=master)](https://github.com/apache/beam/actions?query=workflow%3A%22Java+Tests%22+branch%3Amaster+event%3Aschedule)
   [![Go tests](https://github.com/apache/beam/actions/workflows/go_tests.yml/badge.svg?event=schedule&&?branch=master)](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 or the [workflows README](https://github.com/apache/beam/blob/master/.github/workflows/README.md) to see a list of phrases to trigger workflows.
   


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

Re: [PR] implement main pcollection as default to dooutputstuple [beam]

Posted by "gemini-code-assist[bot] (via GitHub)" <gi...@apache.org>.
gemini-code-assist[bot] commented on code in PR #35754:
URL: https://github.com/apache/beam/pull/35754#discussion_r2245990047


##########
sdks/python/apache_beam/typehints/typed_pipeline_test.py:
##########
@@ -239,6 +239,27 @@ def process(self, element):
     with self.assertRaises(ValueError):
       _ = res['undeclared tag']
 
+  def test_typed_dofn_multi_output_default_main(self):
+    """
+    Test that the main pcollection of a multi-output pcoll pardo
+    is used by default when a PTransform is applied to it.
+    """
+    class NoEvenTagMyDoFn(beam.DoFn):
+      def process(self, element):
+        if element % 2:
+          yield beam.pvalue.TaggedOutput('odd', element)
+        else:
+          yield element
+
+    with beam.Pipeline() as p:
+      res = (
+          p
+          | beam.Create([1, 2, 3])
+          | beam.ParDo(NoEvenTagMyDoFn(tag_even=False)).with_outputs('odd'))

Review Comment:
   ![critical](https://www.gstatic.com/codereviewagent/critical.svg)
   
   The `NoEvenTagMyDoFn` class is instantiated with a `tag_even=False` keyword argument. However, the class does not define an `__init__` method that accepts this argument, which will cause a `TypeError` when the test is run. Please remove this unexpected argument.
   
   ```suggestion
             | beam.ParDo(NoEvenTagMyDoFn()).with_outputs('odd'))
   ```



##########
sdks/python/apache_beam/pvalue.py:
##########
@@ -254,6 +254,9 @@ def __init__(
     # Dictionary of PCollections already associated with tags.
     self._pcolls: Dict[Optional[str], PCollection] = {}
 
+  def __or__(self, other):
+    return self[self._main_tag].__or__(other)

Review Comment:
   ![high](https://www.gstatic.com/codereviewagent/high-priority.svg)
   
   This `print` statement appears to be for debugging purposes. It should be removed before this change is merged to avoid polluting stdout during runs.
   
   ```suggestion
       return self[self._main_tag].__or__(other)
   ```



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

Re: [PR] implement main pcollection as default to dooutputstuple [beam]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #35754: implement main pcollection as default to dooutputstuple
URL: https://github.com/apache/beam/pull/35754


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

Re: [PR] implement main pcollection as default to dooutputstuple [beam]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #35754:
URL: https://github.com/apache/beam/pull/35754#issuecomment-3432191902

   This pull request has been closed due to lack of activity. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.


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

Re: [PR] implement main pcollection as default to dooutputstuple [beam]

Posted by "gemini-code-assist[bot] (via GitHub)" <gi...@apache.org>.
gemini-code-assist[bot] commented on code in PR #35754:
URL: https://github.com/apache/beam/pull/35754#discussion_r2245988234


##########
sdks/python/apache_beam/pvalue.py:
##########
@@ -254,6 +254,9 @@ def __init__(
     # Dictionary of PCollections already associated with tags.
     self._pcolls: Dict[Optional[str], PCollection] = {}
 
+  def __or__(self, other):
+    return self[self._main_tag].__or__(other)

Review Comment:
   ![high](https://www.gstatic.com/codereviewagent/high-priority.svg)
   
   This `print` statement appears to be for debugging purposes and should be removed before merging.



##########
sdks/python/apache_beam/typehints/typed_pipeline_test.py:
##########
@@ -239,6 +239,27 @@ def process(self, element):
     with self.assertRaises(ValueError):
       _ = res['undeclared tag']
 
+  def test_typed_dofn_multi_output_default_main(self):
+    """
+    Test that the main pcollection of a multi-output pcoll pardo
+    is used by default when a PTransform is applied to it.
+    """
+    class NoEvenTagMyDoFn(beam.DoFn):
+      def process(self, element):
+        if element % 2:
+          yield beam.pvalue.TaggedOutput('odd', element)
+        else:
+          yield element
+
+    with beam.Pipeline() as p:
+      res = (
+          p
+          | beam.Create([1, 2, 3])
+          | beam.ParDo(NoEvenTagMyDoFn(tag_even=False)).with_outputs('odd'))

Review Comment:
   ![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg)
   
   The `NoEvenTagMyDoFn` does not have an `__init__` method that accepts a `tag_even` argument. While this doesn't cause an error due to the base `DoFn`'s `__init__`, it's misleading and should be removed for clarity.
   
   ```suggestion
             | beam.ParDo(NoEvenTagMyDoFn()).with_outputs('odd'))
   ```



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

Re: [PR] implement main pcollection as default to dooutputstuple [beam]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #35754:
URL: https://github.com/apache/beam/pull/35754#issuecomment-3406249632

   This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@beam.apache.org list. Thank you for your contributions.


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