-
Notifications
You must be signed in to change notification settings - Fork 1
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
Fix * imports #138
Fix * imports #138
Conversation
Hi @nichollsh @timlichtenberg This PR is ready, could you have a look and let me know what you think? Could you test it with some of your workflows to make sure no imports were missed? I don't expect any issues, except if there were implicit transitive dependencies (e.g. A imports * from B, B imports X from C, where A depends on X). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @stefsmeets, I spotted some missing imports. The default config script runs fine for me.
Co-authored-by: Laurent Soucasse <[email protected]>
Co-authored-by: Laurent Soucasse <[email protected]>
Hi @lsoucasse thanks for pointing those out, I fixed the issues. |
This PR fixes all the
from x import *
imports. Avoiding * imports is generally considered a good practice. I usedruff
to help with the formatting, partly addressing #131 as well.To find import issues, you can use:
Prepend
--fix
to sort imports and remove unused imports:I also set up a small pre-commit config so you can automatically have it sort imports on every commit, e.g.:
Closes #136
Addresses #131
Todo