yeandy commented on a change in pull request #16761:
URL: https://github.com/apache/beam/pull/16761#discussion_r801880919



##########
File path: sdks/python/apache_beam/typehints/opcodes.py
##########
@@ -260,6 +262,13 @@ def build_list(state, arg):
     state.stack[-arg:] = [List[reduce(union, state.stack[-arg:], Union[()])]]
 
 
+def build_set(state, arg):
+  if arg == 0:
+    state.stack.append(List[Union[()]])

Review comment:
       Why are we appending a `List` instead of `Set` to the stack?

##########
File path: sdks/python/apache_beam/typehints/trivial_inference_test.py
##########
@@ -308,13 +329,15 @@ def fn(x1, x2, *unused_args):
       return x1, x2
 
     self.assertReturnType(
-        typehints.Tuple[str, float],
+        typehints.Tuple[typehints.Union[str, float, int],
+                        typehints.Union[str, float, int]],

Review comment:
       I see the more robust typehinting in action here for tuple unpacking. 
Nice! 

##########
File path: sdks/python/apache_beam/typehints/trivial_inference.py
##########
@@ -227,6 +227,9 @@ def element_type(hint):
     return hint.inner_type
   elif isinstance(hint, typehints.TupleHint.TupleConstraint):
     return typehints.Union[hint.tuple_types]
+  elif isinstance(hint,
+                  typehints.UnionHint.UnionConstraint) and not 
hint.union_types:

Review comment:
       Can you help me better understand why we're also requiring the `not 
hint.union_types` part?




-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to