Skip to content

[torch/utils][Code Clean] Clean asserts in torch/utils/*.py #165410

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
Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

[torch/utils][Code Clean] Clean asserts in torch/utils/*.py #165410

wants to merge 3 commits into from

Conversation

KarhouTam
Copy link
Contributor

@KarhouTam KarhouTam commented Oct 14, 2025 •
edited by pytorch-bot bot
Loading

@pytorch-bot PyTorch Bot
Copy link

pytorch-bot bot commented Oct 14, 2025 •
edited
Loading

🔗 Helpful Links

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

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

❌ 1 New Failure, 1 Cancelled Job, 32 Pending, 1 Unrelated Failure

As of commit f777fe6 with merge base c5972eb (image):

NEW FAILURE - The following job has failed:

CANCELLED JOB - The following job was cancelled. Please retry:

FLAKY - The following job failed but was likely due to flakiness present on trunk:

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

@pytorch-bot pytorch-bot bot added the module: cpu CPU specific problem (e.g., perf, algorithm) label Oct 14, 2025
@KarhouTam
Copy link
Contributor Author

@pytorchbot label "module: python frontend" "module: error checking" "topic: not user facing"

@pytorch-bot pytorch-bot bot added module: error checking Bugs related to incorrect/lacking error checking module: python frontend For issues relating to PyTorch's Python frontend topic: not user facing topic category labels Oct 14, 2025
@KarhouTam
Copy link
Contributor Author

cc: @FFFrog @albanD PTAL

@KarhouTam KarhouTam mentioned this pull request Oct 14, 2025
Open
2 tasks
albanD
albanD reviewed Oct 15, 2025
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.

A few small things, but looks pretty good overall!

torch/utils/_config_module.py Outdated
or self.env_name_force is not None
):
raise AssertionError(
"if alias is set, none of {default, justknob and env var} can be set"
Copy link
Collaborator

Choose a reason for hiding this comment

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

missing f ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No. The message is as is. If an error is raised here, both the if-branch and the message are shown and user can know, oh one of

self.default is not _UNSET_SENTINEL
                or self.justknob is not None
                or self.env_name_default is not None
                or self.env_name_force is not None

is not fulfilled. BTW, most of them are None when error raised.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ho I read too fast, my bad!

@KarhouTam
Copy link
Contributor Author

@albanD Thanks for your reviewing! All mentioned nits are fixed. PTAL again.

@mikaylagawarecki mikaylagawarecki added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Oct 17, 2025
albanD
albanD previously approved these changes Oct 20, 2025
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.

Looks great!

@albanD
Copy link
Collaborator

albanD commented Oct 20, 2025

@pytorchbot merge

@pytorch-bot pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Oct 20, 2025
@pytorchmergebot
Copy link
Collaborator

Merge started

Your 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

Advanced Debugging
Check the merge workflow status
here

@pytorchmergebot
Copy link
Collaborator

This PR (#165410) was merged in e20c9bf but it is still open, likely due to a Github bug, so mergebot is closing it manually. If you think this is a mistake, please feel free to reopen and contact Dev Infra.

c00w pushed a commit to c00w/pytorch that referenced this pull request Oct 20, 2025
@KarhouTam @pytorchmergebot
[torch/utils][Code Clean] Clean asserts in torch/utils/*.py (pytorc…
e20c9bf 
…h#165410)

Including:
- `torch/utils/*.py`

Fixes part of pytorch#164878

Pull Request resolved: pytorch#165410
Approved by: https://github.com/albanD
Chao1Han pushed a commit to Chao1Han/pytorch that referenced this pull request Oct 21, 2025
@KarhouTam @Chao1Han
[torch/utils][Code Clean] Clean asserts in torch/utils/*.py (pytorc…
519678c 
…h#165410)

Including:
- `torch/utils/*.py`

Fixes part of pytorch#164878

Pull Request resolved: pytorch#165410
Approved by: https://github.com/albanD
@clee2000
Copy link
Contributor

@pytorchbot revert -m "sorry I'm going to revert this since I want to try to back out some other things that are conflicting with this, there is nothing wrong with this PR, rebasing and resolving the merge conflicts should be enough, sorry for the churn" -c weird

@pytorchmergebot
Copy link
Collaborator

@pytorchbot successfully started a revert job. Check the current status here.
Questions? Feedback? Please reach out to the PyTorch DevX Team

pytorchmergebot added a commit that referenced this pull request Oct 21, 2025
@pytorchmergebot
Revert "[torch/utils][Code Clean] Clean asserts in torch/utils/*.py (…
bdf7cb9 
…#165410)"

This reverts commit e20c9bf.

Reverted #165410 on behalf of https://github.com/clee2000 due to sorry I'm going to revert this since I want to try to back out some other things that are conflicting with this, there is nothing wrong with this PR, rebasing and resolving the merge conflicts should be enough, sorry for the churn ([comment](#165410 (comment)))
@pytorchmergebot
Copy link
Collaborator

@KarhouTam your PR has been successfully reverted.

@pytorchmergebot pytorchmergebot added Reverted ci-no-td Do not run TD on this PR labels Oct 21, 2025
@pytorch-bot pytorch-bot bot dismissed albanD’s stale review October 21, 2025 16:28

This PR was reopened (likely due to being reverted), so your approval was removed. Please request another review.

@pytorch-bot pytorch-bot bot removed the ciflow/trunk Trigger trunk jobs on your pull request label Oct 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@albanD albanD albanD left review comments

@XuehaiPan XuehaiPan Awaiting requested review from XuehaiPan XuehaiPan is a code owner

@fmassa fmassa Awaiting requested review from fmassa fmassa is a code owner

@ezyang ezyang Awaiting requested review from ezyang ezyang is a code owner

@malfet malfet Awaiting requested review from malfet malfet is a code owner

Assignees

No one assigned

Labels

ci-no-td Do not run TD on this PR Merged module: cpu CPU specific problem (e.g., perf, algorithm) module: error checking Bugs related to incorrect/lacking error checking module: python frontend For issues relating to PyTorch's Python frontend open source Reverted topic: not user facing topic category triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@KarhouTam @albanD @pytorchmergebot @clee2000 @pytorchbot @mikaylagawarecki