Cover common isinstance calling patterns with some overloads#15954
Cover common isinstance calling patterns with some overloads#15954jonathandung wants to merge 3 commits into
isinstance calling patterns with some overloads#15954Conversation
|
Diff from mypy_primer, showing the effect of this PR on open source code: mypy (https://github.com/python/mypy)
+ mypy/types.py:4277: error: Argument 2 to "isinstance" has incompatible type "tuple[type[TypeAliasType], type[UnionType]]"; expected "type[Never] | tuple[type[Never]]" [arg-type]
+ mypy/types.py:4277: note: Error code "arg-type" not covered by "type: ignore[misc]" comment
+ mypy/memprofile.py:39: error: Argument 2 to "isinstance" has incompatible type "tuple[type[Node], type[Type]]"; expected "type[Never] | tuple[type[Never]]" [arg-type]
+ mypy/memprofile.py:39: note: Error code "arg-type" not covered by "type: ignore[misc]" comment
artigraph (https://github.com/artigraph/artigraph)
- src/arti/internal/type_hints.py:139: error: Argument 2 has incompatible type "type[T@get_item_from_annotated]"; expected "T@lenient_issubclass" [arg-type]
+ src/arti/internal/type_hints.py:139: error: Cannot call function of unknown type [operator]
pydantic (https://github.com/pydantic/pydantic)
+ pydantic/_internal/_typing_extra.py:192: error: Argument 2 to "isinstance" has incompatible type "tuple[type[GenericAlias], Any]"; expected "type[Never] | tuple[type[Never]]" [arg-type]
sphinx (https://github.com/sphinx-doc/sphinx)
+ sphinx/util/inspect.py: note: In function "isgenericalias":
+ sphinx/util/inspect.py:451:28: error: Argument 2 to "isinstance" has incompatible type "tuple[type[GenericAlias], Any]"; expected "type[Never] | tuple[type[Never]]" [arg-type]
+ sphinx/util/inspect.py:451:28: note: Error code "arg-type" not covered by "type: ignore[attr-defined]" comment
mongo-python-driver (https://github.com/mongodb/mongo-python-driver)
+ pymongo/synchronous/pool.py:381: error: No overload variant of "isinstance" matches argument types "MutableMapping[str, Any]", "Sequence[type[Any]]" [call-overload]
+ pymongo/synchronous/pool.py:381: note: Error code "call-overload" not covered by "type: ignore[arg-type]" comment
+ pymongo/synchronous/pool.py:381: note: Possible overload variants:
+ pymongo/synchronous/pool.py:381: note: def [_T] isinstance(object, type[_T] | tuple[type[_T]], /) -> TypeIs[_T]
+ pymongo/synchronous/pool.py:381: note: def [_T, _T1] isinstance(object, tuple[type[_T], type[_T1]], /) -> TypeIs[_T | _T1]
+ pymongo/synchronous/pool.py:381: note: def [_T, _T1, _T2] isinstance(object, tuple[type[_T], type[_T1], type[_T2]], /) -> TypeIs[_T | _T1 | _T2]
+ pymongo/synchronous/pool.py:381: note: def [_T, _T1, _T2, _T3] isinstance(object, tuple[type[_T], type[_T1], type[_T2], type[_T3]], /) -> TypeIs[_T | _T1 | _T2 | _T3]
+ pymongo/synchronous/pool.py:381: note: def [_T, _T1, _T2, _T3, _T4] isinstance(object, tuple[type[_T], type[_T1], type[_T2], type[_T3], type[_T4]], /) -> TypeIs[_T | _T1 | _T2 | _T3 | _T4]
+ pymongo/synchronous/pool.py:381: note: def [_T, _T1, _T2, _T3, _T4, _T5] isinstance(object, tuple[type[_T], type[_T1], type[_T2], type[_T3], type[_T4], type[_T5]], /) -> TypeIs[_T | _T1 | _T2 | _T3 | _T4 | _T5]
+ pymongo/synchronous/pool.py:381: note: def isinstance(object, _ClassInfo, /) -> bool
+ pymongo/asynchronous/pool.py:381: error: No overload variant of "isinstance" matches argument types "MutableMapping[str, Any]", "Sequence[type[Any]]" [call-overload]
+ pymongo/asynchronous/pool.py:381: note: Error code "call-overload" not covered by "type: ignore[arg-type]" comment
+ pymongo/asynchronous/pool.py:381: note: Possible overload variants:
+ pymongo/asynchronous/pool.py:381: note: def [_T] isinstance(object, type[_T] | tuple[type[_T]], /) -> TypeIs[_T]
+ pymongo/asynchronous/pool.py:381: note: def [_T, _T1] isinstance(object, tuple[type[_T], type[_T1]], /) -> TypeIs[_T | _T1]
+ pymongo/asynchronous/pool.py:381: note: def [_T, _T1, _T2] isinstance(object, tuple[type[_T], type[_T1], type[_T2]], /) -> TypeIs[_T | _T1 | _T2]
+ pymongo/asynchronous/pool.py:381: note: def [_T, _T1, _T2, _T3] isinstance(object, tuple[type[_T], type[_T1], type[_T2], type[_T3]], /) -> TypeIs[_T | _T1 | _T2 | _T3]
+ pymongo/asynchronous/pool.py:381: note: def [_T, _T1, _T2, _T3, _T4] isinstance(object, tuple[type[_T], type[_T1], type[_T2], type[_T3], type[_T4]], /) -> TypeIs[_T | _T1 | _T2 | _T3 | _T4]
+ pymongo/asynchronous/pool.py:381: note: def [_T, _T1, _T2, _T3, _T4, _T5] isinstance(object, tuple[type[_T], type[_T1], type[_T2], type[_T3], type[_T4], type[_T5]], /) -> TypeIs[_T | _T1 | _T2 | _T3 | _T4 | _T5]
+ pymongo/asynchronous/pool.py:381: note: def isinstance(object, _ClassInfo, /) -> bool
|
|
Unfortunately, it seems that mypy doesn't like the tuple overloads, for unknown reasons. Maybe the easiest first step is to just support the non-tuple version and leave all tuple variants as |
Unfortunate indeed. But if I can at least have the most common case covered, I'm happy😄 |
|
The mongo-python-driver diff is irrelevant (just an error code change). |
|
We shouldn't merge this unless there's a clear benefit that can be articulated (e.g., code that previously was incorrectly rejected by type checkers). The cost is more complexity and risk of regressions; type checkers all special case |
|
They only narrow on conditional branches, making inlining the check a necessity. Without TypeIs, you can't "transport" (idk if that's the best word to use here) the type information from one function to another, making isinstance de facto useless with closures.
Without this, you are required to write one-liners functions just to wrap the isinstance call if you need the TypeIs, which is annoying, hurt readability, and performance due to the added function call. |
|
I saw that motivation in the issue, but we need to show that this change actually helps with such patterns. This is subtle enough that we should add test cases in the test cases directory. |
Closes #15844.