mypy icon indicating copy to clipboard operation
mypy copied to clipboard

Fix `--strict-equality` for iteratively visited code

Open tyralla opened this issue 5 months ago • 19 comments

Fixes #19328 Fixes #20294

The logic is very similar to what we did to report different revealed types that were discovered in multiple iteration steps in one line. I think this fix is the last one needed before I can implement #19256.

tyralla avatar Aug 10 '25 10:08 tyralla

Diff from mypy_primer, showing the effect of this PR on open source code:

optuna (https://github.com/optuna/optuna)
+ optuna/visualization/_slice.py:231: error: Non-overlapping equality check (left operand type: "float", right operand type: "Literal['None']")  [comparison-overlap]
+ optuna/visualization/matplotlib/_slice.py:118: error: Non-overlapping equality check (left operand type: "float", right operand type: "Literal['None']")  [comparison-overlap]

github-actions[bot] avatar Aug 10 '25 10:08 github-actions[bot]

The primer results are a little mysterious. I could simplify to:

for y in [1.0]:
    if y is not None or y != "None":  # E: Non-overlapping equality check (left operand type: "float", right operand type: "Literal['None']")
        ...

This obvious error is only reported with my change, but I do not know why current master misses it and therefore have no idea what in this PR contributes to detecting it.

For whatever reason this happens, I will add a corresponding test case.

tyralla avatar Aug 10 '25 19:08 tyralla

Diff from mypy_primer, showing the effect of this PR on open source code:

optuna (https://github.com/optuna/optuna)
+ optuna/visualization/_slice.py:231: error: Non-overlapping equality check (left operand type: "float", right operand type: "Literal['None']")  [comparison-overlap]
+ optuna/visualization/matplotlib/_slice.py:118: error: Non-overlapping equality check (left operand type: "float", right operand type: "Literal['None']")  [comparison-overlap]

github-actions[bot] avatar Aug 10 '25 19:08 github-actions[bot]

Ah, I got it. It is because optuna enables --strict-equality but not unreachable. y is not None is always true, so that the y != "None" mismatch is not reported.

I could invest some time so that

for y in [1.0]:
    if y is not None or y != "None":
        ...

and

y = 1.0
if y is not None or y != "None":
    ...

would be handled identically again (no --strict-equality warnings). However, this is a good example that failing to raise warnings in unreachable code can be confusing, and there is already a PR that tries to improve the situation (#18707). Hence, maybe no further action is required here.

tyralla avatar Aug 10 '25 21:08 tyralla

Diff from mypy_primer, showing the effect of this PR on open source code:

core (https://github.com/home-assistant/core)
+ homeassistant/components/homekit/__init__.py:982: error: Unused "type: ignore" comment  [unused-ignore]

urllib3 (https://github.com/urllib3/urllib3)
+ src/urllib3/connectionpool.py:1056: error: Unused "type: ignore" comment  [unused-ignore]

werkzeug (https://github.com/pallets/werkzeug)
+ src/werkzeug/datastructures/accept.py:259: error: Unused "type: ignore" comment  [unused-ignore]
+ src/werkzeug/datastructures/accept.py:264: error: Unused "type: ignore" comment  [unused-ignore]

github-actions[bot] avatar Aug 11 '25 06:08 github-actions[bot]

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

github-actions[bot] avatar Aug 11 '25 19:08 github-actions[bot]

I revisited the problem and added two lines of code that reset the old behaviour for the discussed problem and make some sense to me. The problem was that "nearer" error watchers could not filter non-overlap reports anymore. Now, the collection mechanism of IterationErrorWatcher only applies if no nearer error watcher has _filter activated. I think this is the place where the now again respected error watcher is created:

https://github.com/python/mypy/blob/5a786075d8c366ee753c62fa36857589023ed561/mypy/checkexpr.py#L5982-L5999

tyralla avatar Aug 11 '25 20:08 tyralla

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

github-actions[bot] avatar Aug 23 '25 22:08 github-actions[bot]

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

github-actions[bot] avatar Sep 07 '25 19:09 github-actions[bot]

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

github-actions[bot] avatar Oct 09 '25 20:10 github-actions[bot]

@ilevkivskyi: I don't want to overstretch your willingness to help, but could you also take a look at this one? This PR aligns closely with what we began in #19324 and would enable me to implement reachability checking for functions with constrained type variables.

tyralla avatar Oct 12 '25 12:10 tyralla

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

github-actions[bot] avatar Oct 31 '25 23:10 github-actions[bot]

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

github-actions[bot] avatar Nov 15 '25 08:11 github-actions[bot]

This fixes #20294 too.

gschaffner avatar Nov 24 '25 05:11 gschaffner

This fixes #20294 too.

Thanks for checking this. I edited the initial comment so we don't forget to close this issue when merging this PR.

tyralla avatar Nov 24 '25 17:11 tyralla

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

github-actions[bot] avatar Nov 24 '25 17:11 github-actions[bot]

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

github-actions[bot] avatar Nov 28 '25 21:11 github-actions[bot]

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

github-actions[bot] avatar Dec 05 '25 20:12 github-actions[bot]

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

github-actions[bot] avatar Dec 05 '25 21:12 github-actions[bot]

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

github-actions[bot] avatar Dec 14 '25 21:12 github-actions[bot]

(oh, you also need to update tests to use new union syntax)

ilevkivskyi avatar Dec 14 '25 22:12 ilevkivskyi

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

github-actions[bot] avatar Dec 15 '25 21:12 github-actions[bot]

(oh, you also need to update tests to use new union syntax)

Also done.

LG, but I am a bit worried about performance. Did you make any measurements with misc/perf_compare.py?

Sorry, I should have done this already. I now performed two runs:

master                    7.990s (0.0%) | stdev 0.315s
fix_strict_quality_in_loops 7.876s (-1.4%) | stdev 0.283s
Total time taken by the whole benchmarking program (including any setup): 11 minutes, 4 seconds
master                    7.894s (0.0%) | stdev 0.415s
fix_strict_quality_in_loops 7.931s (+0.5%) | stdev 0.459s
Total time taken by the whole benchmarking program (including any setup): 11 minutes, 5 seconds

tyralla avatar Dec 15 '25 22:12 tyralla

@ilevkivskyi and @sterliakov : Thanks for the reviews!

tyralla avatar Dec 15 '25 22:12 tyralla