-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
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
TYP: enable reportUnusedImport #46937
Conversation
This pull request is stale because it has been open for thirty days with no activity. Please update and respond to this comment if you're still interested in working on this. |
This generally looked okay if you want to merge main. |
…reportUnsupportedDunderAll)
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.
So just to confirm:
- Nothing changed from the "public" API?
- In terms of code flow, we will need to ensure all "unused" imports are defined in
__all__
?
Technically, users will import fewer modules if they use
flake8/pyright will let you know about unused imports :) They can then either be removed, be ignored (if they have some side effects), or added to |
Could you do a quick before/after check when |
Seems there is actually no differences between main and this PR when imported everything from a file: import importlib
from pathlib import Path
import pickle
import ipdb
imported = {}
for py in Path("pandas").glob("**/*.py"):
print(py)
py = str(py).replace("/", ".").removesuffix(".py")
mod = importlib.import_module("py")
imported[py] = [x for x in dir(mod) if not x.startswith("_")]
with open("before.pickle", mode="wb") as file:
pickle.dump(imported, file)
# ran the above first on this PR and save as after.pickle
# and then the entire script on main
after = {}
with open("after.pickle", mode="rb") as file:
after = pickle.load(file)
for py, imports_b in imported.items():
imports_a = set(after[py])
imports_b = set(imports_b)
remove = imports_b.difference(imports_a)
added = imports_a.difference(imports_b)
if len(remove) > 0 or len(added) > 0:
ipdb.set_trace() # never ended up here |
Thanks for double checking @twoertwein! |
* TYP: enable reportUnusedImport * cannot instantiate Tick+BaseOffset * added new import to __all__ * remove references to now non-existing functions (found by pytest and reportUnsupportedDunderAll)
pyright's reportUnusedImport checks py and pyi files for unused imports. If an import is explicitly marked as public (re-exported using
as
or in__all__
) it is "used" (flake8 seems to use the same definition).Adding unused imports to
__all__
becomes messy when the file does not yet have__all__
: need to list also all public symbols (constants, classes, functions).