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

Disable mypy overload-overlap warnings? #12178

Open
srittau opened this issue Jun 20, 2024 · 6 comments · May be fixed by #12470
Open

Disable mypy overload-overlap warnings? #12178

srittau opened this issue Jun 20, 2024 · 6 comments · May be fixed by #12470
Labels
project: infrastructure typeshed build, test, documentation, or distribution related

Comments

@srittau
Copy link
Collaborator

srittau commented Jun 20, 2024

These mypy warnings are overzealous, as it's often quite useful to have partial overlaps: We ignore this warning in 192 places in typeshed at the moment, and I don't think we gain any benefit from having it enabled.

Ideally, mypy would have a separate warning for overloads that are completely shadowed by another overload, though.

@AlexWaygood
Copy link
Member

We should check how many become unused with the mypy master branch, as the way the lint works at mypy has just been completely reworked in python/mypy#17392.

@srittau
Copy link
Collaborator Author

srittau commented Jun 20, 2024

That sounds promising!

@AlexWaygood
Copy link
Member

AlexWaygood commented Jun 20, 2024

So yes, these are mypy's new complaints regarding overload-overlap if you use mypy master, fiddle with mypy's master branch so that you get rid of the thing where it stops --warn-unused-ignores from working inside typeshed directories, then fiddle with our mypy_test.py so that it ignores the override error code (because there's a bunch of new errors from that error code on mypy master which are unrelated to the thing we're discussing here), and then run python tests/mypy_test.py stdlib -p3.13:

stdlib/typing.pyi:1056: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/types.pyi:322: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/types.pyi:573: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/re.pyi:77: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/re.pyi:127: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/re.pyi:133: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/re.pyi:139: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/re.pyi:158: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/re.pyi:164: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/re.pyi:175: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/importlib/metadata/__init__.pyi:49: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/enum.pyi:108: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/enum.pyi:244: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/collections/__init__.pyi:289: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/builtins.pyi:171: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/builtins.pyi:1735: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/builtins.pyi:1745: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/tempfile.pyi:464: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/tempfile.pyi:474: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/urllib/parse.pyi:201: error: Overloaded function signatures 1 and 2 overlap with incompatible return types  [overload-overlap]
stdlib/urllib/parse.pyi:207: error: Overloaded function signatures 1 and 2 overlap with incompatible return types  [overload-overlap]
stdlib/unittest/mock.pyi:278: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/turtle.pyi:339: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/turtle.pyi:385: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/turtle.pyi:396: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/turtle.pyi:620: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/turtle.pyi:659: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/turtle.pyi:669: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/tkinter/ttk.pyi:1043: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/tkinter/ttk.pyi:1087: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/urllib/error.pyi:17: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/multiprocessing/sharedctypes.pyi:76: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/multiprocessing/sharedctypes.pyi:118: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/multiprocessing/sharedctypes.pyi:122: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/multiprocessing/sharedctypes.pyi:123: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/asyncio/tasks.pyi:151: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/asyncio/tasks.pyi:153: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/asyncio/tasks.pyi:157: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/asyncio/tasks.pyi:166: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/asyncio/tasks.pyi:176: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/asyncio/tasks.pyi:189: error: Unused "type: ignore" comment  [unused-ignore]

And in our third-party stubs we have:

stubs/WebOb/webob/request.pyi:44: error: Unused "type: ignore" comment  [unused-ignore]
stubs/gevent/gevent/timeout.pyi:46: error: Unused "type: ignore" comment  [unused-ignore]
stubs/icalendar/icalendar/parser_tools.pyi:11: error: Unused "type: ignore" comment  [unused-ignore]
stubs/icalendar/icalendar/parser_tools.pyi:15: error: Unused "type: ignore" comment  [unused-ignore]
stubs/olefile/olefile/olefile.pyi:168: error: Unused "type: ignore" comment  [unused-ignore]
stubs/olefile/olefile/olefile.pyi:170: error: Unused "type: ignore" comment  [unused-ignore]
stubs/openpyxl/openpyxl/xml/_functions_overloads.pyi:80: error: Unused "type: ignore" comment  [unused-ignore]
stubs/openpyxl/openpyxl/descriptors/nested.pyi:40: error: Unused "type: ignore" comment  [unused-ignore]
stubs/openpyxl/openpyxl/descriptors/nested.pyi:158: error: Unused "type: ignore" comment  [unused-ignore]
stubs/openpyxl/openpyxl/descriptors/nested.pyi:274: error: Unused "type: ignore" comment  [unused-ignore]
stubs/pika/pika/adapters/twisted_connection.pyi:26: error: Unused "type: ignore" comment  [unused-ignore]
stubs/pika/pika/adapters/twisted_connection.pyi:27: error: Unused "type: ignore" comment  [unused-ignore]
stubs/pycocotools/pycocotools/mask.pyi:25: error: Unused "type: ignore" comment  [unused-ignore]
stubs/setuptools/pkg_resources/__init__.pyi:127: error: Unused "type: ignore" comment  [unused-ignore]
stubs/setuptools/pkg_resources/__init__.pyi:136: error: Unused "type: ignore" comment  [unused-ignore]
stubs/setuptools/pkg_resources/__init__.pyi:155: error: Unused "type: ignore" comment  [unused-ignore]
stubs/setuptools/pkg_resources/__init__.pyi:159: error: Unused "type: ignore" comment  [unused-ignore]
stubs/setuptools/pkg_resources/__init__.pyi:196: error: Unused "type: ignore" comment  [unused-ignore]
stubs/ExifRead/exifread/utils.pyi:9: error: Unused "type: ignore" comment  [unused-ignore]

So in total that's around 60 that will go away altogether. That will still leave us with quite a few errors ignored in typeshed, and I agree we don't get much value from the check. (We already have pyright's version switched off, because it was even noisier than mypy's.)

Diff I made to mypy to get --warn-unused-ignore working:

--- a/mypy/errors.py
+++ b/mypy/errors.py
@@ -680,11 +680,6 @@ class Errors:
                 self.has_blockers.remove(path)
 
     def generate_unused_ignore_errors(self, file: str) -> None:
-        if (
-            is_typeshed_file(self.options.abs_custom_typeshed_dir if self.options else None, file)
-            or file in self.ignored_files
-        ):
-            return
         ignored_lines = self.ignored_lines[file]
         used_ignored_lines = self.used_ignored_lines[file]
         for line, ignored_codes in ignored_lines.items():

Diff I made to our tests:

diff --git a/tests/mypy_test.py b/tests/mypy_test.py
index cafaca811..431439509 100755
--- a/tests/mypy_test.py
+++ b/tests/mypy_test.py
@@ -270,6 +270,8 @@ def run_mypy(
             "ignore-without-code",
             "--enable-error-code",
             "redundant-self",
+            "--disable-error-code",
+            "override",
             "--config-file",
             temp.name,
         ]

@srittau
Copy link
Collaborator Author

srittau commented Jun 20, 2024

My gut feeling says that we should disable the test even after the PR (as you also wrote), but maybe it's a good idea to do this after the next mypy release, as a baseline.

@srittau srittau added the project: infrastructure typeshed build, test, documentation, or distribution related label Aug 1, 2024
srittau added a commit to srittau/typeshed that referenced this issue Aug 1, 2024
@srittau srittau linked a pull request Aug 1, 2024 that will close this issue
@hauntsaninja
Copy link
Collaborator

Tbh, I kind of like these, despite the fact that we often have no choice but to ignore them. When reading stubs, they alert me to useful facts about the way overloads are structured. I would vote we prune down the ignores to match the state following python/mypy#17392 which gets rid of some of the least helpful ones (I'll open a PR soon so we can take a look).

@hauntsaninja
Copy link
Collaborator

I'm actually now quite strongly opposed to disabling these warnings, given the spectrum of views in https://discuss.python.org/t/draft-typing-spec-chapter-for-overloads . Until we have specified behaviour of overloads, I think it's very useful to clearly call out areas where typeshed's needs in practice can inform that specification

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
project: infrastructure typeshed build, test, documentation, or distribution related
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants