-
-
Notifications
You must be signed in to change notification settings - Fork 18.1k
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 reportGeneralTypeIssues but add many exceptions #44855
Conversation
ifi you can rebase (merged the other one) |
Here is also a quick and ugly script to first remove all the reportGeneralTypeIssues, run pyright, and add the needed reportGeneralTypeIssues back. from pathlib import Path
# usage:
# first run with add = False (remove all reportGeneralTypeIssues)
# then run pyright > log (get a current list of violations)
# and then add = True (add needed exceptions)
add = False
# used to focus on only some lines of the pyright output
dir_prefix = "/path/to/the/pandas/directory"
if add:
# read log and add exceptions
counts = {}
for file in sorted(
x.split(":", 1)[0].strip()
for x in Path("log").read_text().split("\n")
if "error:" in x
):
if not file.startswith(dir_prefix) or not Path(file).is_file():
continue
file = file.split("pandas/", 1)[1]
if file not in counts:
counts[file] = 0
counts[file] += 1
for file, count in sorted(counts.items(), key=lambda x: x[1]):
path = Path(file)
if path.is_file():
content = path.read_text()
if "reportGeneralTypeIssues = false" not in content:
content = "# pyright: reportGeneralTypeIssues = false\n" + content
path.write_text(content)
print(file, "\t", count)
else:
# remove all reportGeneralTypeIssues exceptions
remove = "# pyright: reportGeneralTypeIssues = false\n"
for file in Path("pandas").glob("**/*.py"):
content = file.read_text()
if remove not in content:
continue
file.write_text(content.replace(remove, "")) |
If this PR touched too many files, I'm happy to wait until all parts of #43744 are merged (interval.pyi #44922 and offset.pyi are missing). These PRs make many classes resolvable by mypy/pyright (currently they resolve it as Any and pyright also creates an error for things it cannot find). Unfortunately, these PRs are not guaranteed to reduce the number of errors, as they might uncover new type errors. |
yeah let's merge those first (then rebase this and we can quickly merge) |
I will put this PR into draft mode. It will probably take some time to have all components of #43744 merged.
|
@twoertwein if its better to merge this can do that too first |
Should at last wait until mypy 0.930 is released python/mypy#11700 (comment) and integrated in the CI. Then we can also bump pyright before merging this PR. (We could already use the latest version of pyright, but we would need to clone and old version of typeshed and point pyright to it.) |
@twoertwein is there a viable way to break this into smaller pieces? 155 files is pretty daunting |
I don't think so. Pyright does not allow as flexible configurations as mypy. I'm happy to wait (or also close this PR). One big difference between mypy and pyright (that I initially wasn't aware of) is that pyright tries to infer missing return annotations: which can uncover typing bugs. |
I think it might be best to convert this PR into an issue and then over time try to reduce the number of files that need exceptions. |
Closing this for now. I hope that we can enable reportGeneralTypeIssues once return values are tightened with overloads (mypy assumes Any but pyright infers the type which often leads to Unions). I will focus on PRs that will slowly decrease the number of exceptions for reportGeneralTypeIssues. |
Calling pyright with |
reportGeneralTypeIssues is probably the most important option in pyright. This PR enables it but disables it in all files that currently have at least one incompatibility with it.
edit: merging #44251 would reduce the number of exceptions a bit. A good number of violations are because of incomplete pyi files.
Here is a list of files and how many errors they have at the moment. I will put that in an issue: