Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add None return type to init #132335

Closed
wants to merge 5 commits into from
Closed

Add None return type to init #132335

wants to merge 5 commits into from

Conversation

oulgen
Copy link
Contributor

@oulgen oulgen commented Jul 31, 2024 •
edited
Loading

@oulgen
Add None return type to init
306b581 
[ghstack-poisoned]
@pytorch-bot PyTorch Bot
Copy link

pytorch-bot bot commented Jul 31, 2024 •
edited
Loading

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/132335

Note: Links to docs will display an error until the docs builds have been completed.

✅ No Failures

As of commit fcfc442 with merge base 043e41f (image):
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@pytorch-bot pytorch-bot bot added ciflow/inductor module: cpu CPU specific problem (e.g., perf, algorithm) module: distributed_checkpoint module: dynamo module: inductor oncall: distributed Add this issue/PR to distributed oncall triage queue labels Jul 31, 2024
oulgen added a commit that referenced this pull request Jul 31, 2024
@oulgen
Add None return type to init
fc17758 
ghstack-source-id: 9dd7fd69512f66f1d87e674d0989ace80b1d866c
Pull Request resolved: #132335
albanD
albanD approved these changes Jul 31, 2024
View reviewed changes
Copy link
Collaborator

@albanD albanD left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not.
This will regress unless we add lint though.

@oulgen
Copy link
Contributor Author

oulgen commented Jul 31, 2024

@albanD yeah, ideally we remove all the mypy ignores.

I did this via

fastmod "def __init__\(self\):$" "def __init__(self) -> None:" torch

so if this regresses, it is ok.

@oulgen oulgen added ciflow/trunk Trigger trunk jobs on your pull request topic: not user facing topic category labels Jul 31, 2024
@oulgen
Update on "Add None return type to init"
273bd46 
cc XilunWu H-Huang awgu kwen2501 wanchaol fegin fduwjj wz337 wconstab d4l3k c-p-i-o jgong5 mingfeima XiaobingSuper sanchitintel ashokei jingxu10 voznesenskym penguinwu EikanWang Guobing-Chen zhuhaozhe blzheng wenzhe-nrv jiayisunx ipiszy yf225 chenyang78 kadeng muchulee8 ColinPeppler amjames desertfire chauhang LucasLLC MeetVadakkanchery mhorowitz

[ghstack-poisoned]
oulgen added a commit that referenced this pull request Jul 31, 2024
@oulgen
Add None return type to init
d733b03 
ghstack-source-id: 8aa50782aa1bb0b413fdb5d54fd0bdf377647727
Pull Request resolved: #132335
This was referenced Jul 31, 2024
Closed
Closed
oulgen added 2 commits July 31, 2024 17:41
@oulgen
Update on "Add None return type to init"
aaba251 
cc XilunWu H-Huang awgu kwen2501 wanchaol fegin fduwjj wz337 wconstab d4l3k c-p-i-o jgong5 mingfeima XiaobingSuper sanchitintel ashokei jingxu10 voznesenskym penguinwu EikanWang Guobing-Chen zhuhaozhe blzheng wenzhe-nrv jiayisunx ipiszy yf225 chenyang78 kadeng muchulee8 ColinPeppler amjames desertfire chauhang LucasLLC MeetVadakkanchery mhorowitz

[ghstack-poisoned]
@oulgen
Update on "Add None return type to init"
36ac900 
cc XilunWu H-Huang awgu kwen2501 wanchaol fegin fduwjj wz337 wconstab d4l3k c-p-i-o jgong5 mingfeima XiaobingSuper sanchitintel ashokei jingxu10 voznesenskym penguinwu EikanWang Guobing-Chen zhuhaozhe blzheng wenzhe-nrv jiayisunx ipiszy yf225 chenyang78 kadeng muchulee8 ColinPeppler amjames desertfire chauhang LucasLLC MeetVadakkanchery mhorowitz

[ghstack-poisoned]
@oulgen oulgen mentioned this pull request Aug 1, 2024
Closed
@oulgen
Copy link
Contributor Author

oulgen commented Aug 1, 2024

@pytorchbot merge -f "ci appears to be stuck"

@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged immediately since you used the force (-f) flag, bypassing any CI checks (ETA: 1-5 minutes). Please use -f as last resort and instead consider -i/--ignore-current to continue the merge ignoring current failures. This will allow currently pending tests to finish and report signal before the merge.

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging
Check the merge workflow status
here

@pytorchmergebot
Copy link
Collaborator

Merge failed

Reason: Command git -C /home/runner/work/pytorch/pytorch cherry-pick -x 57e209d58b5c9f393f2a9a0a83838a0ac8614ca7 returned non-zero exit code 1

Auto-merging torch/_dynamo/variables/builder.py
Auto-merging torch/_guards.py
Auto-merging torch/_inductor/codegen/common.py
Auto-merging torch/_inductor/codegen/cpp_wrapper_cuda.py
Auto-merging torch/_inductor/codegen/triton.py
Auto-merging torch/_inductor/dependencies.py
CONFLICT (content): Merge conflict in torch/_inductor/dependencies.py
Auto-merging torch/distributed/_composable/fsdp/_fsdp_state.py
error: could not apply 57e209d58b... Add None return type to init
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".
hint: Disable this message with "git config advice.mergeConflict false"
Details for Dev Infra team Raised by workflow job

@oulgen
Update on "Add None return type to init"
fcfc442 
cc XilunWu H-Huang awgu kwen2501 wanchaol fegin fduwjj wz337 wconstab d4l3k c-p-i-o jgong5 mingfeima XiaobingSuper sanchitintel ashokei jingxu10 voznesenskym penguinwu EikanWang Guobing-Chen zhuhaozhe blzheng wenzhe-nrv jiayisunx ipiszy yf225 chenyang78 kadeng muchulee8 ColinPeppler amjames desertfire chauhang LucasLLC MeetVadakkanchery mhorowitz

[ghstack-poisoned]
@XuehaiPan
Copy link
Collaborator

I did this via

fastmod "def __init__\(self\):$" "def __init__(self) -> None:" torch

so if this regresses, it is ok.

You can try this:

ruff check --select=ANN204 --fix-only --fix --unsafe-fixes torch

It will add return type for __init__, __eq__, __ne__, __len__, __hash__, __str__, __repr__, __int__, __float__, __setitem__, __setattr__.

See also: https://docs.astral.sh/ruff/rules/missing-return-type-special-method

pytorchmergebot pushed a commit that referenced this pull request Aug 1, 2024
@oulgen @pytorchmergebot
Add None return type to init -- functorch and torchgen (#132351)
a6985c0 
Pull Request resolved: #132351
Approved by: https://github.com/jamesjwu
ghstack dependencies: #132335
pytorchmergebot pushed a commit that referenced this pull request Aug 1, 2024
@oulgen @pytorchmergebot
Add None return type to init -- tests (#132352)
221350e 
Pull Request resolved: #132352
Approved by: https://github.com/ezyang
ghstack dependencies: #132335, #132351
pytorchmergebot pushed a commit that referenced this pull request Aug 1, 2024
@oulgen @pytorchmergebot
Add None return type to init -- tests rest (#132376)
920f042 
Pull Request resolved: #132376
Approved by: https://github.com/jamesjwu
ghstack dependencies: #132335, #132351, #132352
@github-actions github-actions bot deleted the gh/oulgen/109/head branch September 1, 2024 02:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@albanD albanD albanD approved these changes

@eqy eqy Awaiting requested review from eqy

@syed-ahmed syed-ahmed Awaiting requested review from syed-ahmed

@avikchaudhuri avikchaudhuri Awaiting requested review from avikchaudhuri

@tugsbayasgalan tugsbayasgalan Awaiting requested review from tugsbayasgalan

@zhxchen17 zhxchen17 Awaiting requested review from zhxchen17

@ydwu4 ydwu4 Awaiting requested review from ydwu4

@angelayi angelayi Awaiting requested review from angelayi

@jeffdaily jeffdaily Awaiting requested review from jeffdaily

@jithunnair-amd jithunnair-amd Awaiting requested review from jithunnair-amd

@andrewkho andrewkho Awaiting requested review from andrewkho

@gokulavasan gokulavasan Awaiting requested review from gokulavasan

@aaronenyeshi aaronenyeshi Awaiting requested review from aaronenyeshi

@sraikund16 sraikund16 Awaiting requested review from sraikund16

@mruberry mruberry Awaiting requested review from mruberry

@titaiwangms titaiwangms Awaiting requested review from titaiwangms

@shubhambhokare1 shubhambhokare1 Awaiting requested review from shubhambhokare1

@justinchuby justinchuby Awaiting requested review from justinchuby

@wschin wschin Awaiting requested review from wschin

@jbschlosser jbschlosser Awaiting requested review from jbschlosser

@mikaylagawarecki mikaylagawarecki Awaiting requested review from mikaylagawarecki

Assignees
No one assigned
Labels
ciflow/inductor ciflow/trunk Trigger trunk jobs on your pull request Merged module: cpu CPU specific problem (e.g., perf, algorithm) module: distributed_checkpoint module: dynamo module: inductor oncall: distributed Add this issue/PR to distributed oncall triage queue release notes: AO frontend release notes: quantization release notes category topic: not user facing topic category
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@oulgen @pytorchmergebot @XuehaiPan @albanD