The Wayback Machine - https://web.archive.org/web/20220215062043/https://github.com/pytorch/pytorch/pull/55838
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.

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

Un-ignore F403 in .flake8 #55838

Closed
wants to merge 5 commits into from
Closed

Un-ignore F403 in .flake8 #55838

wants to merge 5 commits into from

Conversation

@samestep
Copy link
Contributor

@samestep samestep commented Apr 12, 2021 •
edited

Generally wildcard imports are bad for the reasons described here: https://www.flake8rules.com/rules/F403.html

This PR replaces wildcard imports with an explicit list of imported items where possible, and adds a # noqa: F403 comment in the other cases (mostly re-exports in __init__.py files).

This is a prerequisite for #55816, because currently tools/codegen/dest/register_dispatch_key.py simply fails if you sort its imports.

Test plan:

CI. You can also run flake8 locally.
@facebook-github-bot
Copy link
Contributor

@facebook-github-bot facebook-github-bot commented Apr 12, 2021 •
edited

💊 CI failures summary and remediations

As of commit 4f4e096 (more details on the Dr. CI page):

✅ None of the CI failures appear to be your fault 💚

🚧 2 fixed upstream failures:

These were probably caused by upstream breakages that were already fixed.

Please rebase on the viable/strict branch (expand for instructions)

If your commit is older than viable/strict, run these commands:

git fetch https://github.com/pytorch/pytorch viable/strict
git rebase FETCH_HEAD
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 to the (internal) Dr. CI Users group.

@samestep samestep changed the title Un-ignore F403 in .flake8 Remove wildcard imports Apr 12, 2021
@samestep samestep changed the title Remove wildcard imports Un-ignore F403 in .flake8 Apr 12, 2021
@facebook-github-bot
Copy link
Contributor

@facebook-github-bot facebook-github-bot commented Apr 12, 2021

@samestep has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@samestep samestep marked this pull request as ready for review Apr 12, 2021
@samestep samestep requested a review from Apr 12, 2021
janeyx99
janeyx99 approved these changes Apr 13, 2021
View changes
Copy link
Contributor

@janeyx99 janeyx99 left a comment

Would the ones where you added # noqa: F403 still block the isort change?

@samestep
Copy link
Contributor Author

@samestep samestep commented Apr 13, 2021

Would the ones where you added # noqa: F403 still block the isort change?

@janeyx99 I don't believe so, but I could open a draft PR based on top of this one and use that to check

@samestep samestep requested a review from Apr 13, 2021
@janeyx99
Copy link
Contributor

@janeyx99 janeyx99 commented Apr 13, 2021

Would the ones where you added # noqa: F403 still block the isort change?

@janeyx99 I don't believe so, but I could open a draft PR based on top of this one and use that to check

Yea feel free to do so in a separate PR

@facebook-github-bot
Copy link
Contributor

@facebook-github-bot facebook-github-bot commented Apr 13, 2021

@samestep merged this pull request in 4753100.

@samestep samestep mentioned this pull request Apr 13, 2021
Closed
@samestep samestep deleted the unignore-f403 branch Apr 14, 2021
krshrimali pushed a commit to krshrimali/pytorch that referenced this issue May 19, 2021
@samestep Kushashwa Shrimali
Un-ignore F403 in .flake8 (pytorch#55838)
15f3232 
Summary:
Generally wildcard imports are bad for the reasons described here: https://www.flake8rules.com/rules/F403.html

This PR replaces wildcard imports with an explicit list of imported items where possible, and adds a `# noqa: F403` comment in the other cases (mostly re-exports in `__init__.py` files).

This is a prerequisite for pytorch#55816, because currently [`tools/codegen/dest/register_dispatch_key.py` simply fails if you sort its imports](https://github.com/pytorch/pytorch/actions/runs/742505908).

Pull Request resolved: pytorch#55838

Test Plan: CI. You can also run `flake8` locally.

Reviewed By: jbschlosser

Differential Revision: D27724232

Pulled By: samestep

fbshipit-source-id: 269fb09cb4168f8a51fd65bfaacc6cda7fb87c34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@janeyx99 janeyx99

@albanD albanD

@BowenBao BowenBao

@H-Huang H-Huang

@jbschlosser jbschlosser

@mingzhe09088 mingzhe09088

@mrshenli mrshenli

@neginraoof neginraoof

@pritamdamania87 pritamdamania87

@rohan-varma rohan-varma

@wayi1 wayi1

@soulitzer soulitzer

@spandantiwari spandantiwari

@zhaojuanmao zhaojuanmao

@pytorch/pytorch-dev-infra PyTorch Dev Infra

Assignees
No one assigned
Labels
cla signed Merged
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet
3 participants
@samestep @facebook-github-bot @janeyx99