-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[BEAM-8280] Enable type hint annotations #10717
Conversation
3018d07
to
cc854f5
Compare
Run Portable_Python PreCommit |
Run PythonFormatter PreCommit |
Run Python PreCommit |
Adds `@decorators.no_annotations` decorator to disable on a specific function. Split out of apache#10717
469319e
to
cccd4b6
Compare
Run Python PreCommit |
Run Python2_PVR_Flink PreCommit |
R: @robertwb |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice.
fb3230b
to
e8d2844
Compare
Changes default to use type annotations. Adds `apache_beam.typehints.disable_type_annotations()`, which disables annotations globally.
e8d2844
to
636de82
Compare
Run Python PreCommit |
1 similar comment
Run Python PreCommit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was looking into it while triaging failing import.
@@ -138,8 +139,7 @@ def foo((a, b)): | |||
|
|||
_ANY_VAR_POSITIONAL = typehints.Tuple[typehints.Any, ...] | |||
_ANY_VAR_KEYWORD = typehints.Dict[typehints.Any, typehints.Any] | |||
# TODO(BEAM-8280): Remove this when from_callable is ready to be enabled. | |||
_enable_from_callable = False | |||
_disable_from_callable = False |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be True? (based on the check in line 234)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, I inverted the logic and made using type annotations on by default.
Changes default to use type annotations.
Adds a couple of way to disable them:
apache_beam.typehints.disable_type_annotations()
to disable globally.@apache_beam.typehints.no_annotations
decorator to disable on a specificfunction.
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
R: @username
).[BEAM-XXX] Fixes bug in ApproximateQuantiles
, where you replaceBEAM-XXX
with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.See the Contributor Guide for more tips on how to make review process smoother.
Post-Commit Tests Status (on master branch)
Pre-Commit Tests Status (on master branch)
See .test-infra/jenkins/README for trigger phrase, status and link of all Jenkins jobs.