Skip to content
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

Revisit the body of a loop if the number of partial types has changed. #18180

Merged

Conversation

tyralla
Copy link
Collaborator

@tyralla tyralla commented Nov 23, 2024

Fixes #5423

tyralla and others added 3 commits November 24, 2024 00:14
# Conflicts:
#	test-data/unit/check-narrowing.test
for more information, see https://pre-commit.ci

This comment has been minimized.

fix

This comment has been minimized.

@hauntsaninja
Copy link
Collaborator

hauntsaninja commented Nov 24, 2024

I went through the primer and this is looking great. 3-5 cases where ideally mypy would do something different, but may be technically unrelated to this PR (didn't look).

operator: true positive
*prefect: false positive, seems like we should narrow from tuple to str on line 220
manticore: true positive
*pylint: false positive, can't tell why mypy wants an explicit annotation
xarray: true positive from typing perspective (typeguard guarantee isn't encoded in signature)
sympy: true positive, should assert
comtypes: true positive, heterogeneous list that is mutated so can't be a tuple[int, x, y, z]
aiortc: true positive, not sure where _data comes from, is maybe patched on
*httpx-caching: false positive, should infer int | None or require explicit annotation
jax: true positive, true positive
flake8: true positive, should assert
*poetry: probably should infer Term | None or require an explicit annotation
dulwich: true positive, should assert
*pyodide: false positive, not sure what's going on there

@hauntsaninja hauntsaninja requested a review from JukkaL November 24, 2024 01:03
@tyralla
Copy link
Collaborator Author

tyralla commented Nov 24, 2024

@hauntsaninja: Thank you very much for looking so thoroughly through the primer!

Copy link
Contributor

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

operator (https://github.com/canonical/operator)
+ ops/lib/__init__.py:219: error: Argument 1 to "_Missing" has incompatible type "dict[str | Any, object]"; expected "bool"  [arg-type]

manticore (https://github.com/trailofbits/manticore)
+ tests/wasm/json2mc.py:103: error: Invalid index type "int | None" for "list[Module]"; expected type "SupportsIndex"  [index]

prefect (https://github.com/PrefectHQ/prefect)
+ src/prefect/settings/legacy.py:105: note:     Expected:
+ src/prefect/settings/legacy.py:105: note:         def __hash__() -> int
+ src/prefect/settings/legacy.py:105: note:     Got:
+ src/prefect/settings/legacy.py:105: note:         def __hash__(self: object) -> int
+ src/prefect/cli/cloud/__init__.py:224: error: Incompatible return value type (got "str | tuple[Hashable, str]", expected "str")  [return-value]

pylint (https://github.com/pycqa/pylint)
+ pylint/lint/message_state_handler.py:381: error: Need type annotation for "_ignore_file"  [var-annotated]

xarray (https://github.com/pydata/xarray)
+ xarray/core/indexing.py: note: In member "__init__" of class "VectorizedIndexer":
+ xarray/core/indexing.py:488: error: Item "_arrayfunction[Any, Any]" of "_arrayfunction[Any, Any] | _arrayapi[Any, Any]" has no attribute "ndim"  [union-attr]
+ xarray/core/indexing.py:488: error: Item "_arrayapi[Any, Any]" of "_arrayfunction[Any, Any] | _arrayapi[Any, Any]" has no attribute "ndim"  [union-attr]

sympy (https://github.com/sympy/sympy)
+ sympy/tensor/array/expressions/from_array_to_matrix.py:398: error: Invalid index type "int | None" for "list[Any]"; expected type "SupportsIndex"  [index]
+ sympy/tensor/array/expressions/from_array_to_matrix.py:401: error: Invalid index type "int | None" for "list[Any]"; expected type "SupportsIndex"  [index]

comtypes (https://github.com/enthought/comtypes)
+ comtypes/tools/codegenerator/codegenerator.py:848: error: Unsupported operand types for | ("object" and "int")  [operator]
+ comtypes/tools/codegenerator/codegenerator.py:850: error: Unsupported operand types for | ("object" and "int")  [operator]

aiortc (https://github.com/aiortc/aiortc)
+ src/aiortc/jitterbuffer.py:81: error: "RtpPacket" has no attribute "_data"  [attr-defined]

httpx-caching (https://github.com/johtso/httpx-caching)
+ httpx_caching/_policy.py:491: error: Incompatible types in assignment (expression has type "None", target has type "int")  [assignment]

flake8 (https://github.com/pycqa/flake8)
+ src/flake8/processor.py:217: error: Unsupported operand types for - ("None" and "int")  [operator]
+ src/flake8/processor.py:217: note: Left operand is of type "int | None"

jax (https://github.com/google/jax)
+ jax/_src/interpreters/pxla.py:2249: error: Item "AbstractMesh" of "Mesh | AbstractMesh" has no attribute "abstract_mesh"  [union-attr]
+ jax/_src/interpreters/pxla.py:2253: error: Item "AbstractMesh" of "Mesh | AbstractMesh" has no attribute "abstract_mesh"  [union-attr]
+ jax/_src/checkify.py:262: error: Value of type "int | Array" is not indexable  [index]

poetry (https://github.com/python-poetry/poetry)
+ src/poetry/mixology/partial_solution.py:195: error: Incompatible types in assignment (expression has type "Term | None", variable has type "Assignment | None")  [assignment]

pyodide (https://github.com/pyodide/pyodide)
+ src/py/pyodide/_state.py:21: error: Incompatible types in assignment (expression has type "pyodide._state.<subclass of "ModuleType" and "JsProxy">1", target has type "pyodide._state.<subclass of "ModuleType" and "JsProxy">")  [assignment]

dulwich (https://github.com/dulwich/dulwich)
+ dulwich/config.py:558: error: Unsupported left operand type for + ("None")  [operator]
+ dulwich/config.py:558: note: Left operand is of type "bytes | None"
+ dulwich/config.py:560: error: Unsupported left operand type for + ("None")  [operator]
+ dulwich/config.py:560: note: Left operand is of type "bytes | None"
+ dulwich/config.py:562: error: Unsupported left operand type for + ("None")  [operator]
+ dulwich/config.py:562: note: Left operand is of type "bytes | None"

@tyralla
Copy link
Collaborator Author

tyralla commented Nov 24, 2024

I had a look at the false positive for prefect. It seems in fact unrelated to this PR, as it is reproducible without involving loops:

from typing import Union, Tuple

def f() -> bool: ...

u: Union[int, Tuple[int]]
x = None
if f():
    x = u
    if isinstance(x, tuple):
        x = x[0]
reveal_type(x)  # N: Revealed type is "Union[builtins.int, Tuple[builtins.int], None]" 

(Tuple[builtins.int] should be narrowed away.)

Copy link
Member

@ilevkivskyi ilevkivskyi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fix makes sense to me. I will merge soon unless there are objections. (Please ping me if I forget about this PR)

@hauntsaninja hauntsaninja merged commit 0901689 into python:master Dec 21, 2024
19 checks passed
tyralla added a commit to tyralla/mypy that referenced this pull request Jan 9, 2025
Fixes python#18348
Fixes python#13973
Fixes python#11612
Fixes python#8721
Fixes python#8865
Fixes python#7204

I manually checked all the listed issues.  Some of them were already partly fixed by python#18180.
hauntsaninja pushed a commit that referenced this pull request Jan 12, 2025

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
…8433)

Fixes #18348
Fixes #13973
Fixes #11612
Fixes #8721
Fixes #8865
Fixes #7204

I manually checked all the listed issues. Some of them were already
partly fixed by #18180.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

False negative involving null checks on optional types
3 participants