[GitHub] [beam] robertwb commented on a change in pull request #11838: [BEAM-9322] Modify the TestStream to output a dict when no output_tags are specified

2020-06-09 Thread GitBox


robertwb commented on a change in pull request #11838:
URL: https://github.com/apache/beam/pull/11838#discussion_r437787906



##
File path: sdks/python/apache_beam/testing/test_stream.py
##
@@ -291,10 +291,10 @@ def expand(self, pbegin):
 assert isinstance(pbegin, pvalue.PBegin)
 self.pipeline = pbegin.pipeline
 if not self.output_tags:
-  self.output_tags = set([None])
+  self.output_tags = {None}

Review comment:
   OK, in that case I'm fine with this.





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:
us...@infra.apache.org




[GitHub] [beam] robertwb commented on a change in pull request #11838: [BEAM-9322] Modify the TestStream to output a dict when no output_tags are specified

2020-06-09 Thread GitBox


robertwb commented on a change in pull request #11838:
URL: https://github.com/apache/beam/pull/11838#discussion_r437702523



##
File path: sdks/python/apache_beam/testing/test_stream.py
##
@@ -291,10 +291,10 @@ def expand(self, pbegin):
 assert isinstance(pbegin, pvalue.PBegin)
 self.pipeline = pbegin.pipeline
 if not self.output_tags:
-  self.output_tags = set([None])
+  self.output_tags = {None}

Review comment:
   If the user explicitly sets the output tags to {None}, they might be 
expecting a dict. (Specifically, they might get a set from elsewhere, and set 
the output tags from that set, and it would be awkward to have to check that 
set to determine how to interpret the result. So in this case I would do
   
   ```
   if not self.output_tags:
 return pvalue.PCollection(self.pipeline, is_bounded=False)
   else:
 return { ... for tag in self.output_tags}
   ```





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:
us...@infra.apache.org