-
Notifications
You must be signed in to change notification settings - Fork 3.6k
Add mypy typing to precision plugins. #6149
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
Conversation
| def backward( | ||
| self, | ||
| lightning_module: LightningModule, | ||
| model: 'LightningModule', |
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.
isn't this API change?
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.
It was named model in all the other plugins and I checked, It is never called by name.
But if we don't do this, MypY complains, that the signature does not match with base class since it has different parameters
pytorch_lightning/plugins/training_type/training_type_plugin.py
Outdated
Show resolved
Hide resolved
Co-authored-by: Akihiro Nitta <[email protected]>
06355f4 to
249563c
Compare
Codecov Report
@@ Coverage Diff @@
## master #6149 +/- ##
======================================
- Coverage 93% 93% -0%
======================================
Files 159 159
Lines 11381 11381
======================================
- Hits 10626 10597 -29
- Misses 755 784 +29 |
| class NativeMixedPrecisionPlugin(MixedPrecisionPlugin): | ||
|
|
||
| def __init__(self): | ||
| def __init__(self) -> None: |
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.
why is it necessary to annotate init with None?
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.
tchaton
left a comment
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.
LGTM !
Co-authored-by: Jirka Borovec <[email protected]> Co-authored-by: Jirka Borovec <[email protected]> Co-authored-by: Akihiro Nitta <[email protected]>
What does this PR do?
This PR should be merged after #6148
Fixes #<issue_number>
Before submitting
PR review
Anyone in the community is free to review the PR once the tests have passed.
Before you start reviewing make sure you have read Review guidelines. In short, see the following bullet-list:
Did you have fun?
Make sure you had fun coding 🙃