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

upgrade linters #7680

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

upgrade linters #7680

wants to merge 3 commits into from

Conversation

pmeier
Copy link
Collaborator

@pmeier pmeier commented Jun 19, 2023

Important file to review is .pre-commit-config.yaml. Everything else are automatic changes. The major ones:

  1. As suspected in remove obsolete transforms tests #7678 (comment), black >= 23 removes blank lines after function definitions, which accounts for most of the removals in this PR.

  2. flake8==6 gained the support to check for individual unused imports. Meaning

    from foo import bar, baz
    
    bar()

    was fine before, but will now be flagged, since baz is unused. Since flake8 doesn't account for (ancient) # type comments, we had a lot of false flags. I've used com2ann to migrate to best practice of using inline annotations.

@mpearce25 I only now remembered that you have #7222 for the flake8 update. I'm sorry that I forgot about it. In case we move forward with my PR here, are you ok if I attribute you as co-author here and close the original PR?

@pytorch-bot
Copy link

pytorch-bot bot commented Jun 19, 2023

🔗 Helpful Links

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

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

✅ 3 Unrelated Failures

As of commit f7aebe8:

BROKEN TRUNK - The following jobs failed but were present on the merge base 22d981f:

👉 Rebase onto the `viable/strict` branch to avoid these failures

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

Comment on lines 18 to 19
- black == 23.3.0
- usort == 1.0.7
Copy link
Member

@NicolasHug NicolasHug Jun 20, 2023

Choose a reason for hiding this comment

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

We can't, unfortunately.
We have to stick to what is used by pyfmt internally

nicolashug@devvm779 ~ ❯❯❯ pyfmt --version
fb-pyfmt built mode/opt (xar) on Python 3.10.9+fb (platform010)
ufmt version 2.0.1
black version 22.12.0
usort version 1.0.5
libcst version 0.4.10

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated the config to match the pyfmt requirements.

@pmeier pmeier added the other if you have no clue or if you will manually handle the PR in the release notes label Jun 29, 2023
@pmeier pmeier marked this pull request as ready for review June 29, 2023 09:16
@pmeier pmeier requested a review from NicolasHug June 29, 2023 09:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla signed code quality other if you have no clue or if you will manually handle the PR in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants