Skip to content

Run pycln as a pre-commit hook in CI #8304

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

Merged
merged 4 commits into from
Jul 18, 2022
Merged

Run pycln as a pre-commit hook in CI #8304

merged 4 commits into from
Jul 18, 2022

Conversation

AlexWaygood
Copy link
Member

pycln is a pre-commit hook to auto-remove unused import statements. The tool is used by several high-profile open-source projects such as Pyodide, Poetry and Rich. It is fully compatible with .pyi files as well as .py files.

@AlexWaygood AlexWaygood marked this pull request as draft July 15, 2022 10:47
@github-actions

This comment has been minimized.

@AlexWaygood AlexWaygood marked this pull request as ready for review July 15, 2022 11:03
@AlexWaygood
Copy link
Member Author

AlexWaygood commented Jul 15, 2022

Here's pycln successfully auto-removing some unused imports that pyright didn't pick up on: 8d0d6d3

Copy link
Collaborator

@srittau srittau left a comment

Choose a reason for hiding this comment

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

LGTM, but I would like to hear feedback of other maintainers.

@github-actions

This comment has been minimized.

1 similar comment
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Copy link
Collaborator

@hauntsaninja hauntsaninja left a comment

Choose a reason for hiding this comment

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

I'm fine with this. Not familiar with pycln, but it looks like it uses libcst, which is promising (unlike isort, which was plagued with bugs for a long time).

@Akuli
Copy link
Collaborator

Akuli commented Jul 15, 2022

Have you tried running pycln on all of our existing stubs? Does it delete anything? If it does, a file like that would be quite annoying to contribute to.

@AlexWaygood
Copy link
Member Author

AlexWaygood commented Jul 15, 2022

Have you tried running pycln on all of our existing stubs? Does it delete anything? If it does, a file like that would be quite annoying to contribute to.

Yes, I ran it on the whole of typeshed before filing this PR.

It made changes to aiohttp, where it picked up a genuine bug (an import wasn't being re-exported when it should have been). I fixed that in the first commit in this PR: b7e0b2d.

It also identified unused imports in stdlib/tokenize.pyi and stdlib/unittest/loader.pyi. pycln auto-fixed these in the third commit in this PR: 8d0d6d3.

So if this PR is merged, the entirety of typeshed should be pycln- compatible.

@github-actions
Copy link
Contributor

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

@srittau srittau merged commit 35616b4 into master Jul 18, 2022
@srittau srittau deleted the pycln branch July 18, 2022 07:27
@q0w q0w mentioned this pull request Jul 22, 2022
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants