-
Notifications
You must be signed in to change notification settings - Fork 7.2k
fix annotations for Python >= 3.8 #5301
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
💊 CI failures summary and remediationsAs of commit c1059bc (more details on the Dr. CI page):
1 failure not recognized by patterns:
This comment was automatically generated by Dr. CI (expand for details).Please report bugs/suggestions to the (internal) Dr. CI Users group. |
This reverts commit b935c83.
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.
These changes are necessary when linting against numpy>=1.22
, which is the latest version for Python >= 3.8. This was not caught by our CI, because we run 3.7 there.
pytorch_dep += "==" + os.getenv("PYTORCH_VERSION") | ||
|
||
requirements = [ | ||
"typing_extensions", |
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.
PyTorch core already hard-depends on it: https://github.com/pytorch/pytorch/blob/be2dc8f2940d3c95941516a811be8c504910d1ea/setup.py#L408-L411
padding: Union[int, List[int], Tuple[int, ...]], | ||
fill: Optional[Union[float, List[float], Tuple[float, ...]]] = 0, | ||
padding_mode: str = "constant", | ||
padding_mode: Literal["constant", "edge", "reflect", "symmetric"] = "constant", |
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.
That's a new one for me, Literally
.
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!
self.labels: Optional[np.ndarray] | ||
if self.split == "train": | ||
self.data, self.labels = self.__loadfile(self.train_list[0][0], self.train_list[1][0]) | ||
self.labels = cast(np.ndarray, self.labels) |
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.
We already have self.labels: Optional[np.ndarray]
above, should we remove one of these?
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 missed that. Let me have a look.
Summary: * run mypy on Python 3.9 * appease mypy * Revert "run mypy on Python 3.9" This reverts commit b935c83. Reviewed By: kazhang Differential Revision: D33927516 fbshipit-source-id: 07741549abeede756d941bdd105f8df3293e179e
No description provided.