[BE]: Fix typing None override other optimizers#153386
[BE]: Fix typing None override other optimizers#153386Skylion007 wants to merge 4 commits intopytorch:mainfrom
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/153386
Note: Links to docs will display an error until the docs builds have been completed. ✅ You can merge normally! (1 Unrelated Failure)As of commit 8f0b493 with merge base dc47295 ( BROKEN TRUNK - The following job failed but were present on the merge base:👉 Rebase onto the `viable/strict` branch to avoid these failures
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
2eb3282 to
89b9559
Compare
89b9559 to
be75382
Compare
| *args, | ||
| **kwargs, | ||
| *args: tuple[Any, ...], | ||
| **kwargs: dict[str, Any], |
There was a problem hiding this comment.
kwargs should be typed Any unless their values are dict. And args should be Any
There was a problem hiding this comment.
@cyyever they can't be expanded unless the keys are string so it should be fine (raises a type error)
There was a problem hiding this comment.
@cyyever Actually, doing [str, Any] is safer and I'm pretty sure preferrable as it can detect those kind of type errors ahead of time
There was a problem hiding this comment.
By Python syntax, their keys are of course strings.
There was a problem hiding this comment.
Yeah so this should be the appropriate typing for kwargs.
|
@pytorchmergebot merge -i |
Merge startedYour change will be merged while ignoring the following 0 checks: Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
|
The merge job was canceled or timed out. This most often happen if two merge requests were issued for the same PR, or if merge job was waiting for more than 6 hours for tests to finish. In later case, please do not hesitate to reissue the merge command |
|
@pytorchmergebot merge -i |
Merge startedYour change will be merged while ignoring the following 0 checks: Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
|
@pytorchbot merge |
|
The merge job was canceled or timed out. This most often happen if two merge requests were issued for the same PR, or if merge job was waiting for more than 6 hours for tests to finish. In later case, please do not hesitate to reissue the merge command |
Merge startedYour change will be merged once all checks pass (ETA 0-4 Hours). Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
Merge failedReason: 1 mandatory check(s) failed. The first few are: Dig deeper by viewing the failures on hud |
|
@pytorchbot merge -i |
Merge startedYour change will be merged while ignoring the following 1 checks: pull / cuda12.4-py3.10-gcc9-sm75 / test (pr_time_benchmarks, 1, 1, linux.g4dn.metal.nvidia.gpu) Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
Fix typo from #153386 Pull Request resolved: #153561 Approved by: https://github.com/albanD
Follow up to #153367 to fix other instances of it throughout the codebase
Also fully type NamedOptimizer since we were so close
cc @H-Huang @awgu @wanchaol @fegin @fduwjj @wz337 @wconstab @d4l3k @ezyang @malfet @xuzhao9 @gramster