gemini-code-assist[bot] commented on code in PR #36776:
URL: https://github.com/apache/beam/pull/36776#discussion_r2511385855
##########
sdks/python/apache_beam/transforms/ptransform_test.py:
##########
@@ -1402,6 +1402,106 @@ def process(self, element, five):
assert_that(d, equal_to([6, 7, 8]))
self.p.run()
+ def test_child_with_both_input_and_output_hints_binds_typevars_correctly(
+ self):
+ """
+ When a child transform has both input and output type hints with type
+ variables, those variables bind correctly from the actual input data.
+
+ Example: Child with .with_input_types(Tuple[K, V])
+ .with_output_types(Tuple[K, V]) receiving Tuple['a', 'hello'] will bind
+ K=str, V=str correctly.
+ """
+ K = typehints.TypeVariable('K')
+ V = typehints.TypeVariable('V')
+
+ @typehints.with_input_types(typehints.Tuple[K, V])
+ @typehints.with_output_types(typehints.Tuple[K, V])
+ class TransformWithoutChildHints(beam.PTransform):
+ class MyDoFn(beam.DoFn):
+ def process(self, element):
+ k, v = element
+ yield (k, v.upper())
+
+ def expand(self, pcoll):
+ return (
+ pcoll
+ | beam.ParDo(self.MyDoFn()).with_input_types(
+ tuple[K, V]).with_output_types(tuple[K, V]))
+
+ with TestPipeline() as p:
+ result = (
+ p
+ | beam.Create([('a', 'hello'), ('b', 'world')])
+ | TransformWithoutChildHints())
+
+ self.assertEqual(result.element_type, typehints.Tuple[str, str])
+
+ def test_child_without_input_hints_fails_to_bind_typevars(self):
+ """
+ When a child transform lacks input type hints, type variables in its output
+ hints cannot bind and default to Any, even when parent composite has
+ decorated type hints.
+
+ This test demonstrates the current limitation: without explicit input hints
+ on the child, the type variable K in .with_output_types(Tuple[K, str])
+ remains unbound, resulting in Tuple[Any, str] instead of the expected
+ Tuple[str, str].
+ """
+ K = typehints.TypeVariable('K')
+
+ @typehints.with_input_types(typehints.Tuple[K, str])
+ @typehints.with_output_types(typehints.Tuple[K, str])
+ class TransformWithoutChildHints(beam.PTransform):
+ class MyDoFn(beam.DoFn):
+ def process(self, element):
+ k, v = element
+ yield (k, v.upper())
+
+ def expand(self, pcoll):
+ return (
+ pcoll
+ | beam.ParDo(self.MyDoFn()).with_output_types(tuple[K, str]))
+
+ with TestPipeline() as p:
+ result = (
+ p
+ | beam.Create([('a', 'hello'), ('b', 'world')])
+ | TransformWithoutChildHints())
+
+ self.assertEqual(result.element_type, typehints.Tuple[typehints.Any,
str])
+
+ def test_child_without_output_hints_infers_partial_types_from_dofn(self):
+ """
+ When a child transform has input hints but no output hints, type inference
+ from the DoFn's process method produces partially inferred types.
+
+ With .with_input_types(Tuple[K, V]) and actual input Tuple['a', 'hello'], K
+ binds to str from the input. However, without output hints, the DoFn's
+ yield statement inference produces Tuple[str, Any].
+ """
+ K = typehints.TypeVariable('K')
+ V = typehints.TypeVariable('K')
Review Comment:
The `TypeVariable` `V` is created with the name 'K', which is the same as
the `TypeVariable` `K`. While `TypeVariable` identity is based on the object
instance and not the name, this is very confusing and could lead to future
errors. It should be named 'V' for clarity.
```python
V = typehints.TypeVariable('V')
```
##########
sdks/python/apache_beam/transforms/ptransform_test.py:
##########
@@ -1402,6 +1402,106 @@ def process(self, element, five):
assert_that(d, equal_to([6, 7, 8]))
self.p.run()
+ def test_child_with_both_input_and_output_hints_binds_typevars_correctly(
+ self):
+ """
+ When a child transform has both input and output type hints with type
+ variables, those variables bind correctly from the actual input data.
+
+ Example: Child with .with_input_types(Tuple[K, V])
+ .with_output_types(Tuple[K, V]) receiving Tuple['a', 'hello'] will bind
+ K=str, V=str correctly.
+ """
+ K = typehints.TypeVariable('K')
+ V = typehints.TypeVariable('V')
+
+ @typehints.with_input_types(typehints.Tuple[K, V])
+ @typehints.with_output_types(typehints.Tuple[K, V])
+ class TransformWithoutChildHints(beam.PTransform):
+ class MyDoFn(beam.DoFn):
+ def process(self, element):
+ k, v = element
+ yield (k, v.upper())
+
+ def expand(self, pcoll):
+ return (
+ pcoll
+ | beam.ParDo(self.MyDoFn()).with_input_types(
+ tuple[K, V]).with_output_types(tuple[K, V]))
+
+ with TestPipeline() as p:
+ result = (
+ p
+ | beam.Create([('a', 'hello'), ('b', 'world')])
+ | TransformWithoutChildHints())
+
+ self.assertEqual(result.element_type, typehints.Tuple[str, str])
+
+ def test_child_without_input_hints_fails_to_bind_typevars(self):
+ """
+ When a child transform lacks input type hints, type variables in its output
+ hints cannot bind and default to Any, even when parent composite has
+ decorated type hints.
+
+ This test demonstrates the current limitation: without explicit input hints
+ on the child, the type variable K in .with_output_types(Tuple[K, str])
+ remains unbound, resulting in Tuple[Any, str] instead of the expected
+ Tuple[str, str].
+ """
+ K = typehints.TypeVariable('K')
+
+ @typehints.with_input_types(typehints.Tuple[K, str])
+ @typehints.with_output_types(typehints.Tuple[K, str])
+ class TransformWithoutChildHints(beam.PTransform):
+ class MyDoFn(beam.DoFn):
+ def process(self, element):
+ k, v = element
+ yield (k, v.upper())
+
+ def expand(self, pcoll):
+ return (
+ pcoll
+ | beam.ParDo(self.MyDoFn()).with_output_types(tuple[K, str]))
+
+ with TestPipeline() as p:
+ result = (
+ p
+ | beam.Create([('a', 'hello'), ('b', 'world')])
+ | TransformWithoutChildHints())
+
+ self.assertEqual(result.element_type, typehints.Tuple[typehints.Any,
str])
+
+ def test_child_without_output_hints_infers_partial_types_from_dofn(self):
+ """
+ When a child transform has input hints but no output hints, type inference
+ from the DoFn's process method produces partially inferred types.
+
+ With .with_input_types(Tuple[K, V]) and actual input Tuple['a', 'hello'], K
+ binds to str from the input. However, without output hints, the DoFn's
+ yield statement inference produces Tuple[str, Any].
Review Comment:
The docstring is slightly incomplete. It says "K binds to str from the
input", but given the input `Tuple['a', 'hello']` and the hint `Tuple[K, V]`,
`V` also binds to `str`. The docstring should reflect this for clarity.
--
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]
