FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Fix and simplify error de-duplication by ilevkivskyi · Pull Request #19247 · python/mypy · GitHub

  1. python
  2. mypy

Repository navigation

Fix and simplify error de-duplication - #19247

Merged
ilevkivskyi merged 4 commits into
python:masterfrom
ilevkivskyi:better-dedup
Jun 10, 2025
ConversationCommits (4)ChecksFiles changed
Merged

Fix and simplify error de-duplication#19247
ilevkivskyi merged 4 commits into
python:masterfrom
ilevkivskyi:better-dedup

Conversation

ilevkivskyi commented Jun 7, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

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.

This comment has been minimized.

Sign in to view

ilevkivskyi commented Jun 7, 2025

Copy link
Copy Markdown
Member Author

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 commented Jun 7, 2025

Copy link
Copy Markdown
Member Author

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

sterliakov commented Jun 7, 2025

Copy link
Copy Markdown
Collaborator

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

This comment has been minimized.

Sign in to view

github-actions Bot commented Jun 9, 2025

Copy link
Copy Markdown
Contributor

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

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

prefect (https://github.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://github.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://github.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://github.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://github.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://github.com/encode/starlette)
- starlette/websockets.py:21: note: "WebSocketDenialResponse" defined here
- starlette/websockets.py:21: note: "WebSocketDenialResponse" defined here

freqtrade (https://github.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://github.com/Rapptz/discord.py)
- ...venv/lib/python3.13/site-packages/mypy/typeshed/stdlib/typing.pyi:1014: note: "update" of "TypedDict" defined here

zulip (https://github.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://github.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 commented Jun 10, 2025

Copy link
Copy Markdown
Member Author

@JukkaL do you have any comments objections to this?

JukkaL approved these changes Jun 10, 2025
View reviewed changes

JukkaL left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Looks good, just a few questions.

Comment thread mypy/messages.py
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)

JukkaL Jun 10, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Should these propagate parent_error?

ilevkivskyi Jun 10, 2025 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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.

Comment thread mypy/messages.py
)
if call:
self.note_call(original_caller_type, call, context, code=code)
self.note_call(original_caller_type, call, context, code=parent_error.code)

JukkaL Jun 10, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Propagate parent_error?

ilevkivskyi merged commit dc42e28 into python:master Jun 10, 2025
ilevkivskyi deleted the better-dedup branch June 10, 2025 16:28
hauntsaninja mentioned this pull request Jun 10, 2025
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

JukkaL JukkaL approved these changes
hauntsaninja Awaiting requested review from hauntsaninja

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

3 participants

Back | FazBrowse Home | New Git URL