Enable type check for torch.testing._internal.common_device_type. #44911
Conversation
facebook-github-bot
left a comment
facebook-github-bot
left a comment
There was a problem hiding this comment.
@wuyangzhang has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
💊 CI failures summary and remediationsAs of commit fe49ee5 (more details on the Dr. CI page): 💚 💚 Looks good so far! There are no failures yet. 💚 💚 This comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.Please report bugs/suggestions on the GitHub issue tracker or post in the (internal) Dr. CI Users group. This comment has been revised 11 times. |
facebook-github-bot
left a comment
facebook-github-bot
left a comment
There was a problem hiding this comment.
@wuyangzhang has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
facebook-github-bot
left a comment
facebook-github-bot
left a comment
There was a problem hiding this comment.
@wuyangzhang has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
mypy.ini
Outdated
| #[mypy-torch.testing._internal.common_device_type.*] | ||
| #ignore_errors = True |
There was a problem hiding this comment.
please remove lines entirely.
| import runpy | ||
| import threading | ||
| from functools import wraps | ||
| import typing |
There was a problem hiding this comment.
using from typing import Any, ... is better since you dont need to type typing.* below.
| # See below for how this list is populated. If you're adding a device type | ||
| # you should check if it's available and (if it is) add it to this list. | ||
| device_type_test_bases = [] | ||
| device_type_test_bases: typing.List[typing.Any] = list() |
There was a problem hiding this comment.
please keep my suggested change and add following comment above this line
# set type to List[Any] due to mypy list-of-union issue: https://github.com/python/mypy/issues/3351
| class_name = generic_test_class.__name__ + base.device_type.upper() | ||
| device_type_test_class = type(class_name, (base, empty_class), {}) | ||
|
|
||
| device_type_test_class: typing.Any = type(class_name, (base, empty_class), {}) |
There was a problem hiding this comment.
Please keep my suggested change and add the following comment above this line
# type set to Any and suppressed due to unsupport runtime class: https://github.com/python/mypy/wiki/Unsupported-Python-Features
| # Todo: Error: Argument 1 to "append" of "list" has incompatible type "Type[CUDATestBase]"; expected "Type[CPUTestBase]" | ||
| # tried to add Union to include both.. |
There was a problem hiding this comment.
get rid of this comment. you dont need this
facebook-github-bot
left a comment
facebook-github-bot
left a comment
There was a problem hiding this comment.
@wuyangzhang has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
facebook-github-bot
left a comment
facebook-github-bot
left a comment
There was a problem hiding this comment.
@wuyangzhang has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
facebook-github-bot
left a comment
facebook-github-bot
left a comment
There was a problem hiding this comment.
@wuyangzhang has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
Codecov Report
@@ Coverage Diff @@
## master #44911 +/- ##
=======================================
Coverage 67.89% 67.89%
=======================================
Files 384 384
Lines 49890 49894 +4
=======================================
+ Hits 33874 33878 +4
Misses 16016 16016
Continue to review full report at Codecov.
|
|
@wuyangzhang merged this pull request in d22dd80. |
4 participants
This PR intends to fix the type exceptions in common_device_type.py.