Conversation

ilevkivskyi

Fixes #19240
Fixes #13517
Fixes #17791

Existing de-duplication logic is both complicated and fragile. By specifying a parent_error for a note explicitly, we can do everything in a more robust and simple way. In addition, this new argument makes error code matching simpler.

@github-actionsGitHub Actions

This comment has been minimized.

@ilevkivskyi

I like the mypy_primer results, apart from fixing the de-duplication logic for protocols and overrides (which is the core goal of this PR), this has also the effect that "note: Foo defined here" is only shown once per line if there are multiple unexpected keyword arguments in the same function call.

@ilevkivskyi

Hm, it looks like it may also fix couple other issues. I will check now, and add tests if yes.

@sterliakov

Results look amazing, this should also fix duplicated compatibility notes in #19168.

@github-actionsGitHub Actions

This comment has been minimized.

@github-actionsGitHub Actions

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

colour (https://.com/colour-science/colour)
+ colour/algebra/tests/test_extrapolation.py:140: note:     Setter types should behave contravariantly

prefect (https://.com/PrefectHQ/prefect)
- src/prefect/settings/legacy.py:136: note:     Expected:
- src/prefect/settings/legacy.py:136: note:         def __hash__() -> int
- src/prefect/settings/legacy.py:136: note:     Got:
- src/prefect/settings/legacy.py:136: note:         def __hash__(self: object) -> int
- ...venv/lib/python3.13/site-packages/mypy/typeshed/stdlib/builtins.pyi:136: note: "__init_subclass__" of "object" defined here
- ...venv/lib/python3.13/site-packages/mypy/typeshed/stdlib/builtins.pyi:136: note: "__init_subclass__" of "object" defined here
- ...venv/lib/python3.13/site-packages/mypy/typeshed/stdlib/builtins.pyi:136: note: "__init_subclass__" of "object" defined here
- ...venv/lib/python3.13/site-packages/mypy/typeshed/stdlib/builtins.pyi:136: note: "__init_subclass__" of "object" defined here

tornado (https://.com/tornadoweb/tornado)
- tornado/test/web_test.py:1539: note:      Superclass:
- tornado/test/web_test.py:1539: note:          @classmethod
- tornado/test/web_test.py:1539: note:          def make_static_url(cls, settings: dict[str, Any], path: str, include_version: bool = ...) -> str
- tornado/test/web_test.py:1539: note:      Subclass:
- tornado/test/web_test.py:1539: note:          @classmethod
- tornado/test/web_test.py:1539: note:          def make_static_url(cls, settings: Any, path: Any) -> Any

ibis (https://.com/ibis-project/ibis)
- ibis/backends/snowflake/__init__.py:464: note:      Superclass:
- ibis/backends/snowflake/__init__.py:464: note:          def to_pandas_batches(self, Expr, /, *, params: Mapping[Scalar, Any] | None = ..., limit: int | str | None = ..., chunk_size: int = ..., **kwargs: Any) -> Iterator[DataFrame | Series[Any] | Any]
- ibis/backends/snowflake/__init__.py:464: note:      Subclass:
- ibis/backends/snowflake/__init__.py:464: note:          def to_pandas_batches(self, Expr, /, *, params: Mapping[Scalar, Any] | None = ..., limit: int | str | None = ..., chunk_size: int = ...) -> Iterator[DataFrame | Series[Any] | Any]
- ibis/backends/mysql/__init__.py:157: note:      Superclass:
- ibis/backends/mysql/__init__.py:157: note:          def list_databases(self, *, like: str | None = ..., catalog: str | None = ...) -> list[str]
- ibis/backends/mysql/__init__.py:157: note:      Subclass:
- ibis/backends/mysql/__init__.py:157: note:          def list_databases(self, *, like: str | None = ...) -> list[str]
- ibis/backends/mysql/__init__.py:222: note:      Superclass:
- ibis/backends/mysql/__init__.py:222: note:          def create_database(self, str, /, *, catalog: str | None = ..., force: bool = ...) -> None
- ibis/backends/mysql/__init__.py:222: note:      Subclass:
- ibis/backends/mysql/__init__.py:222: note:          def create_database(self, name: str, force: bool = ...) -> None
- ibis/backends/impala/__init__.py:206: note:      Superclass:
- ibis/backends/impala/__init__.py:206: note:          def list_databases(self, *, like: str | None = ..., catalog: str | None = ...) -> list[str]
- ibis/backends/impala/__init__.py:206: note:      Subclass:
- ibis/backends/impala/__init__.py:206: note:          def list_databases(self, *, like: str | None = ...) -> list[str]
- ...venv/lib/python3.13/site-packages/mypy/typeshed/stdlib/builtins.pyi:114: note: "__init__" of "object" defined here
- ...venv/lib/python3.13/site-packages/mypy/typeshed/stdlib/builtins.pyi:114: note: "__init__" of "object" defined here

hydpy (https://.com/hydpy-dev/hydpy)
- hydpy/core/netcdftools.py:489: note:     Expected:
- hydpy/core/netcdftools.py:489: note:         def __iter__(self) -> Iterator[Buffer]
- hydpy/core/netcdftools.py:489: note:     Got:
- hydpy/core/netcdftools.py:489: note:         def __iter__(self) -> Iterator[int]

yarl (https://.com/aio-libs/yarl)
+ yarl/_url.py:1266:23: note:     keys
+ yarl/_url.py:1266:23: note: Following member(s) of "str" have conflicts:
+ yarl/_url.py:1266:23: note: Following member(s) of "str" have conflicts:

starlette (https://.com/encode/starlette)
- starlette/websockets.py:21: note: "WebSocketDenialResponse" defined here
- starlette/websockets.py:21: note: "WebSocketDenialResponse" defined here

freqtrade (https://.com/freqtrade/freqtrade)
- .../projects/_freqtrade_venv/lib/python3.13/site-packages/pandas-stubs/core/series.pyi:1314: note: "infer_objects" of "Series" defined here

discord.py (https://.com/Rapptz/discord.py)
- ...venv/lib/python3.13/site-packages/mypy/typeshed/stdlib/typing.pyi:1014: note: "update" of "TypedDict" defined here

zulip (https://.com/zulip/zulip)
- ...venv/lib/python3.13/site-packages/mypy/typeshed/stdlib/builtins.pyi:114: note: "SubTest" defined here
- ...venv/lib/python3.13/site-packages/mypy/typeshed/stdlib/builtins.pyi:114: note: "SubTest" defined here
- ...venv/lib/python3.13/site-packages/mypy/typeshed/stdlib/builtins.pyi:114: note: "SubTest" defined here
- ...venv/lib/python3.13/site-packages/mypy/typeshed/stdlib/builtins.pyi:114: note: "SubTest" defined here
- ...venv/lib/python3.13/site-packages/mypy/typeshed/stdlib/builtins.pyi:114: note: "SubTest" defined here
- ...venv/lib/python3.13/site-packages/mypy/typeshed/stdlib/builtins.pyi:114: note: "SubTest" defined here
- ...venv/lib/python3.13/site-packages/mypy/typeshed/stdlib/builtins.pyi:114: note: "SubTest" defined here
- ...venv/lib/python3.13/site-packages/mypy/typeshed/stdlib/builtins.pyi:114: note: "SubTest" defined here
- ...venv/lib/python3.13/site-packages/mypy/typeshed/stdlib/builtins.pyi:114: note: "SubTest" defined here
- ...venv/lib/python3.13/site-packages/mypy/typeshed/stdlib/builtins.pyi:114: note: "SubTest" defined here
- ...venv/lib/python3.13/site-packages/mypy/typeshed/stdlib/builtins.pyi:114: note: "SubTest" defined here
- ...venv/lib/python3.13/site-packages/mypy/typeshed/stdlib/builtins.pyi:114: note: "SubTest" defined here
- ...venv/lib/python3.13/site-packages/mypy/typeshed/stdlib/builtins.pyi:114: note: "SubTest" defined here
- ...venv/lib/python3.13/site-packages/mypy/typeshed/stdlib/builtins.pyi:114: note: "SubTest" defined here
- ...venv/lib/python3.13/site-packages/mypy/typeshed/stdlib/builtins.pyi:114: note: "SubTest" defined here

sphinx (https://.com/sphinx-doc/sphinx)
- sphinx/transforms/__init__.py: note: In function "apply":
- sphinx/transforms/__init__.py:433:32: error: Call to untyped function "astext" in typed context  [no-untyped-call]
- sphinx/transforms/__init__.py: note: In member "apply" of class "GlossarySorter":
- sphinx/transforms/__init__.py:433:32: error: Call to untyped function "astext" in typed context  [no-untyped-call]
- sphinx/transforms/__init__.py: note: In function "apply":
- sphinx/transforms/__init__.py:433:32: error: Call to untyped function "astext" in typed context  [no-untyped-call]

@ilevkivskyi

@JukkaL do you have any comments objections to this?

Choose a reason for hiding this comment

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

Looks good, just a few questions.

if isinstance(callee_type, Instance) and callee_type.type.is_protocol:
call = find_member("__call__", callee_type, callee_type, is_operator=True)
if call:
self.note_call(callee_type, call, context, code=code)
self.maybe_note_concatenate_pos_args(original_caller_type, callee_type, context, code)
self.note_call(callee_type, call, context, code=parent_error.code)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should these propagate parent_error?

Copy link
Member Author

@ilevkivskyi ilevkivskyi Jun 10, 2025

Choose a reason for hiding this comment

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

This one and the one below are actually intentional, so that if two identical notes from different errors appear on the same line, only first one is shown (since IMO they may be a bit verbose). In contrast, I propagate the parent_error in the contravariance note for setters (even though it means duplication from two different errors), since people are not very familiar with this. I know this is a bit arbitrary, so will be happy to change this in a follow-up PR if you prefer different logic here.

)
if isinstance(callee_type, CallableType) and isinstance(original_caller_type, Instance):
call = find_member(
"__call__", original_caller_type, original_caller_type, is_operator=True
)
if call:
self.note_call(original_caller_type, call, context, code=code)
self.note_call(original_caller_type, call, context, code=parent_error.code)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Propagate parent_error?

@ilevkivskyiilevkivskyi merged commit dc42e28 into python:master Jun 10, 2025
19 checks passed
@ilevkivskyiilevkivskyi deleted the better-dedup branch June 10, 2025 16:28
Sign up for free to join this conversation on . Already have an account? Sign in to comment
None yet
None yet