Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork2.9k
Avoid falseunreachable andredundant-expr warnings in loops more robustly and efficiently, and avoid multiplerevealed type notes for the same line.#19118
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:master
Are you sure you want to change the base?
Conversation
… robustly and efficiently, and avoid multiple `revealed type` notes for the same line.This change is an improvement over9685171. Besides fixing the regression reported inpython#18606 and removing the duplicates reported inpython#18511, it should significantly reduce the performance regression reported inpython#18991. At least running `Measure-command {python runtests.py self}` on my computer (with removed cache) is 10 % faster.
for more information, seehttps://pre-commit.ci
This comment has been minimized.
This comment has been minimized.
@JukkaL: I thought that later responses to Alternatively, I could just remove the |
Diff frommypy_primer, showing the effect of this PR on open source code: xarray (https://github.com/pydata/xarray)+ xarray/conventions.py:413: error: Argument "decode_timedelta" to "decode_cf_variable" has incompatible type "object"; expected "bool | CFTimedeltaCoder | None" [arg-type] |
I implemented a collection mechanism for the types revealed in different iteration steps. In the case of unions, the items are sorted alphabetically without differentiating upper and lower case letters. At first glance, the reason for the Mypy primer diff (https://github.com/pydata/xarray) is not clear to me. I asked for a review of those who opened the mentioned issues. Thanks in advance! |
xarray is#19121, we have it on all current PRs |
@sterliakov: Thanks, good to know. (Anybody already working on it?) |
@tyralla I'm a bit swamped, there are no related PRs open - so probably not yet? It is likely related to#18976, but I have no idea why it's inconsistent - any MRE extracted from that fails every time, not randomly.https://mypy-play.net/?mypy=master&python=3.12&flags=strict&gist=50f4ae811a650f9e90c6f52269f59d29 |
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.
I'm not immediately convinced this is robust!
| # We collect errors that indicate unreachability or redundant expressions | ||
| # in the first iteration and remove them in subsequent iterations if the | ||
| # related statement becomes reachable or non-redundant due to changes in | ||
| # partial types: |
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.
What if an unreachable statement is within another unreachable statement?
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.
I am not exactly sure what you are asking for.
Is this an example that fits your question?
[casetestAvoidUnreachableCodeInUnreachableCodeWithPartialType]# flags: --warn-unreachable --python-version 3.11x=Noney=NonewhilexisNoneandyisNone:reveal_type(x)# N: Revealed type is "None"reveal_type(y)# N: Revealed type is "None"ifxisNoneandyisnotNone:x=1# E: Statement is unreachableifyisNone:y=1[builtinsfixtures/bool.pyi]
In this case, the results are the same for 1.15 and the current state of this PR.
What kind of test case could convince you? |
Fixes#18606
Closes#18511
Improves#18991
This change is an improvement over9685171. Besides fixing the regression reported in#18606 and removing the duplicates reported in#18511, it should significantly reduce the performance regression reported in#18991. At least running
Measure-command {python runtests.py self}on my computer (with removed cache) is 10 % faster.