Normalize types in dataclass field type resolving#38797
Conversation
Abacn
force-pushed
the
fix-normalize
branch
6 times, most recently
from
166eb24 to
6aa5459
Compare
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances Apache Beam's Python type inference by correctly resolving dataclass field types instead of defaulting to 'Any'. To ensure backward compatibility and mitigate potential breaking changes for existing pipelines, a new configuration flag has been added, providing users with a mechanism to opt-out of this stricter type inference. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request enables honoring dataclass field typehints during type inference in the Python SDK, while introducing a --exclude_infer_dataclass_field_type pipeline option for backward compatibility. The review feedback highlights a critical circular import risk in opcodes.py that should be resolved by importing resolve_dataclass_field_type locally. Additionally, the reviewer recommends replacing a print() statement with standard logging in trivial_inference.py and correcting a typo in the pipeline option's help text.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| except AttributeError: | ||
| print(options.view_as(TypeOptions)._visible_option_list()) | ||
| raise |
There was a problem hiding this 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.
| except AttributeError: | |
| print(options.view_as(TypeOptions)._visible_option_list()) | |
| raise | |
| 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 |
| 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 |
There was a problem hiding this comment.
Fix typo: 'senario' should be 'scenario'.
| 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 | |
| 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 |
|
Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control. If you'd like to restart, comment |
Add a pipeline option to allow fallback to Any
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #38797 +/- ##
============================================
- Coverage 56.68% 56.67% -0.01%
Complexity 3637 3637
============================================
Files 1183 1183
Lines 187908 187923 +15
Branches 3751 3751
============================================
+ Hits 106509 106511 +2
- Misses 77933 77946 +13
Partials 3466 3466
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
1 participant
Please add a meaningful description for your change here
internal tracker: b/518498022
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
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, commentfixes #<ISSUE NUMBER>instead.CHANGES.mdwith noteworthy changes.See the Contributor Guide for more tips on how to make review process smoother.
To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md
GitHub Actions Tests Status (on master branch)
See CI.md for more information about GitHub Actions CI or the workflows README to see a list of phrases to trigger workflows.