gemini-code-assist[bot] commented on code in PR #38797:
URL: https://github.com/apache/beam/pull/38797#discussion_r3353014832
##########
sdks/python/apache_beam/options/pipeline_options.py:
##########
@@ -888,6 +888,15 @@ def _add_argparse_args(cls, parser):
default=False,
action='store_true',
help='Disable the use of beartype for type checking.')
+ parser.add_argument(
+ '--exclude_infer_dataclass_field_type',
+ default=False,
+ action='store_true',
+ help='Exclude certain typehint inference involving dataclass fields '
+ 'and resolve to Any (as in beam<=2.74.0). NOTE: this option is '
+ 'for backward compatibility only and the exclusion senario is subject '
+ 'to change, until removing in a future version. For details see: '
+
'https://beam.apache.org/releases/pydoc/current/apache_beam.typehints.trivial_inference.html#apache_beam.typehints.trivial_inference.resolve_dataclass_field_type')
# pylint: disable=line-too-long
Review Comment:
Fix typo: 'senario' should be 'scenario'.
```suggestion
help='Exclude certain typehint inference involving dataclass fields '
'and resolve to Any (as in beam<=2.74.0). NOTE: this option is '
'for backward compatibility only and the exclusion scenario is
subject '
'to change, until removing in a future version. For details see: '
'https://beam.apache.org/releases/pydoc/current/apache_beam.typehints.trivial_inference.html#apache_beam.typehints.trivial_inference.resolve_dataclass_field_type')
# pylint: disable=line-too-long
```
##########
sdks/python/apache_beam/typehints/trivial_inference.py:
##########
@@ -774,3 +774,33 @@ def infer_return_type_func(f, input_types, debug=False,
depth=0):
if debug:
print(f, id(f), input_types, '->', result)
return result
+
+
+def resolve_dataclass_field_type(x):
+ """
+ Resolve a type to Beam typehint under global pipeline option context.
+
+ Since Beam 2.75.0, typehints of dataclass fields are honored during type
+ inferences. However, in case of breakage (possible scenarios include
+ incorrect typehints; non-deterministic or nullable types disallowed by
+ consumer transform but check disabled by Any; tests rely on Any),
+ --exclude_infer_dataclass_field_type option to instruct falling back to Any.
+ Fields of builtin primitives are always respected.
+ """
+ from apache_beam.options.pipeline_options_context import get_pipeline_options
+ options = get_pipeline_options()
+ if options:
+ from apache_beam.options.pipeline_options import TypeOptions
+ try:
+ disabled =
options.view_as(TypeOptions).exclude_infer_dataclass_field_type
+ except AttributeError:
+ print(options.view_as(TypeOptions)._visible_option_list())
+ raise
Review Comment:
Avoid using `print()` in library code as it writes directly to standard
output, which can interfere with user logs or stdout-based tooling. Use the
standard `logging` module instead.
```suggestion
except AttributeError:
import logging
logging.warning(
"Failed to retrieve exclude_infer_dataclass_field_type option. "
"Visible options: %s",
options.view_as(TypeOptions)._visible_option_list())
raise
```
##########
sdks/python/apache_beam/typehints/opcodes.py:
##########
@@ -42,6 +42,7 @@
from apache_beam.typehints.trivial_inference import Const
from apache_beam.typehints.trivial_inference import element_type
from apache_beam.typehints.trivial_inference import key_value_types
+from apache_beam.typehints.trivial_inference import
resolve_dataclass_field_type
from apache_beam.typehints.trivial_inference import union
Review Comment:
Importing `resolve_dataclass_field_type` at the top level of `opcodes.py`
introduces a circular import risk. Since `trivial_inference.py` imports
`opcodes` at the top level, and `resolve_dataclass_field_type` is defined at
the end of `trivial_inference.py`, importing it here will fail with an
`ImportError` during module initialization because the name is not yet defined
when `opcodes` is loaded. Removing this top-level import and importing it
locally where needed resolves this issue.
```suggestion
from apache_beam.typehints.trivial_inference import union
```
##########
sdks/python/apache_beam/typehints/opcodes.py:
##########
@@ -451,8 +452,9 @@ def _getattr(o, name):
elif inspect.isclass(o) and dataclasses.is_dataclass(o):
field = o.__dataclass_fields__.get(name)
if field is not None:
- return field.type
+ return resolve_dataclass_field_type(field.type)
Review Comment:
Import `resolve_dataclass_field_type` locally inside `_getattr` to avoid
circular import issues during module initialization.
```suggestion
from apache_beam.typehints.trivial_inference import
resolve_dataclass_field_type
return resolve_dataclass_field_type(field.type)
```
--
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]
