-
Notifications
You must be signed in to change notification settings - Fork 3.6k
Fixes various typing errors in pytorch_lightning/strategies/deepspeed.py
#13832
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
pytorch_lightning/strategies/deepspeed.py
for more information, see https://pre-commit.ci
Codecov Report
@@ Coverage Diff @@
## master #13832 +/- ##
=========================================
+ Coverage 49% 76% +27%
=========================================
Files 332 332
Lines 26000 26186 +186
=========================================
+ Hits 12728 19848 +7120
+ Misses 13272 6338 -6934 |
|
Hi @donlapark, it seems, that if you remove |
…ng into fixes_typing_deepspeed
for more information, see https://pre-commit.ci
Co-authored-by: Rohit Gupta <[email protected]>
Co-authored-by: Rohit Gupta <[email protected]>
Co-authored-by: Rohit Gupta <[email protected]>
Co-authored-by: Rohit Gupta <[email protected]>
Co-authored-by: Rohit Gupta <[email protected]>
Co-authored-by: Rohit Gupta <[email protected]>
Co-authored-by: Rohit Gupta <[email protected]>
Co-authored-by: Rohit Gupta <[email protected]>
Co-authored-by: Rohit Gupta <[email protected]>
rohitgr7
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.
Great work!
BTW, you can also add all suggestions in a batch and commit them at once inside the PR's Files changed section.
|
Wow, I should have known that. Thanks for the tip! |
awaelchli
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.
Great!
What does this PR do?
Fixes mypy errors in
pytorch_lightning/strategies/deepspeed.pyas addressed in #13445.The code suggests that
lightning_model.trainingis always assumed to have typepl.Trainerand notNone. However,mypydoes not know this and throw out an error wheneverlightning_model.trainingis called.In addition,
mypyseems to not recognizehasattras mentioned in this issue, causing it to throw an error on the following line (671-672) without#type: ignore:Does your PR introduce any breaking changes? If yes, please list them.
Before submitting
PR review
Anyone in the community is welcome to review the PR.
Before you start reviewing, make sure you have read the review guidelines. In short, see the following bullet-list:
Did you have fun?
Make sure you had fun coding 🙃