-
Notifications
You must be signed in to change notification settings - Fork 3.6k
Fix mypy errors attributed to pytorch_lightning/utilities/meta.py
#13763
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
|
While fixing the document, I realized that there is a slight problem with the code file itself. |
pytorch_lightning/utilities/meta.py
Hi @nninept, can you please open a new issue describing it? I have to say, I'm not very familiar with the internals of this file, so with a new issue, there's a better chance, someone more familiar with it will take a look :) |
Sure! I'll open new related issue soon. |
Codecov Report
@@ Coverage Diff @@
## master #13763 +/- ##
=========================================
- Coverage 86% 76% -10%
=========================================
Files 332 332
Lines 26127 26050 -77
=========================================
- Hits 22481 19808 -2673
- Misses 3646 6242 +2596 |
| if isinstance(obj, Exception): | ||
| raise obj |
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.
Are you taking these changes from upstream?
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 wrote this line because init_meta function returns MisconfigurationException when _TORCH_GREATER_EQUAL_1_10 is false.
If the error is returned, obj.materialize would be none. So I thought that it would be better to raise error explicitly
| module_fn: Callable[..., Module], *args: Any, **kwargs: Any | ||
| ) -> Union[Module, MisconfigurationException]: | ||
| if not _TORCH_GREATER_EQUAL_1_10: | ||
| return MisconfigurationException("`init_meta` is supported from PyTorch 1.10.0") |
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.
wondering, why it returns an Exception instead of raising it?
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.
Looks like a bug
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 modified definition line of init_meta function because mypy shows error when conditional function do not have same identical signature.
And original code returns MisconfigurationException, so I followed to return error without additional modify
for more information, see https://pre-commit.ci
What does this PR do?
Fixes mypy typing errors in pytorch_lightning/utilities/meta.py in #13445.
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 🙃