Enable mypy's `disallow_any_explicit` flag. #3121

CoolCat467 · 2024-10-27T06:17:51Z

agreed. disable ANN401 for now and enable `disallow_any_explicit` in a separate PR. Possibly same for `disallow_untyped_defs`.

Originally posted by @jakkdl in #3098 (comment)

In this pull request, we enable mypy's disallow_any_explicit flag. While we are at it, we also upgrade the version of mypy used, as well as enabling a new feature in mypy 1.13.0, where it has support for optional faster cache via a module called orjson.

@jakkdl

agreed. disable ANN401 for now and enable `disallow_any_explicit` in a separate PR. Possibly same for `disallow_untyped_defs`. _Originally posted by @jakkdl in python-trio#3098 (comment)

codecov · 2024-10-27T06:19:17Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.62%. Comparing base (9d4fe27) to head (ba24f74).
Report is 35 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #3121   +/-   ##
=======================================
  Coverage   99.62%   99.62%           
=======================================
  Files         122      122           
  Lines       18340    18380   +40     
  Branches     3281     3281           
=======================================
+ Hits        18271    18311   +40     
  Misses         47       47           
  Partials       22       22

Files with missing lines	Coverage Δ
src/trio/_core/_concat_tb.py	`100.00% <100.00%> (ø)`
src/trio/_core/_entry_queue.py	`100.00% <100.00%> (ø)`
src/trio/_core/_instrumentation.py	`100.00% <100.00%> (ø)`
src/trio/_core/_io_windows.py	`98.80% <100.00%> (ø)`
src/trio/_core/_ki.py	`100.00% <100.00%> (ø)`
src/trio/_core/_run.py	`99.02% <100.00%> (-0.01%)`	⬇️
src/trio/_core/_tests/test_guest_mode.py	`100.00% <100.00%> (ø)`
src/trio/_core/_tests/test_ki.py	`99.77% <100.00%> (ø)`
src/trio/_core/_tests/test_parking_lot.py	`100.00% <100.00%> (ø)`
src/trio/_core/_tests/test_run.py	`100.00% <100.00%> (ø)`
... and 24 more

A5rocks · 2024-10-27T07:03:17Z

Honestly, I don't like the line changes due to having this option enabled. On the other hand, I do like the Any-> object changes you spotted.

I would prefer if we left this error code disabled, but you kept the changes in type signatures if they work. I doubt new type signatures will be added abusing Any to the same level as signatures we added in bulk when typing trio.

TeamSpen210 · 2024-10-27T08:38:38Z

I agree, in the core code there's a lot of passing callables around etc, which just has to be typed as Any. Not sure if there's much point enabling the explicit-any code, then disabling it for each line where we do use Any. Since it's explicit, we can just search for Any itself if we want to audit?

Agreed that we should change Any → object wherever it's possible.

…xplicit

TeamSpen210 · 2024-10-27T22:07:19Z

src/trio/_core/_run.py

@@ -103,8 +103,7 @@ class _NoStatus(metaclass=NoPublicConstructor):

 # Decorator to mark methods public. This does nothing by itself, but
 # trio/_tools/gen_exports.py looks for it.
-# Explicit "Any" is not allowed
-def _public(fn: FnT) -> FnT:  # type: ignore[misc]
+def _public(fn: Callable[PS, RetT]) -> Callable[PS, RetT]:


Actually, this should stay as a typevar bound=Callable[..., object]. There's a subtle difference between that and paramspec - the bound typevar means the callable must be exactly the same from input to output, so things like overloads are preserved. This still might work if type checkers special case it, but it's not clear in the spec how it'll interact with ParamSpec.

I was considering changing to def _public(fn: T) -> T: because that's all it really does, and I think this cements that that might be a better solution.

Changed in 55964ad

I was considering changing to def _public(fn: T) -> T: because that's all it really does, and I think this cements that that might be a better solution.

well, the comment specifies that it's only used "to mark methods public" so using FnT is a way of enforcing that. But it's probably fine to do T->T

CoolCat467 · 2024-10-28T06:44:12Z

pypy cache issue strikes again

A5rocks · 2024-10-28T07:48:36Z

pypy cache issue strikes again

I'm going to make a few commits adding in debugging etc, feel free to remove them via git reset --hard aee4a770ddebb3d631a07e81399a2d1fd8982ece; git push --force later.

A5rocks · 2024-10-28T08:10:11Z

OK, so this time:

the hashes were the same between PyPy and CPython on Windows
the cache was still incorrect

I tried removing the cache from https://github.com/python-trio/trio/actions/caches for PyPy on Windows and that seems to work. Maybe it was just a really evil file that was living far past when it should have and this will never happen again. At least that gives us an easy fix if this happens again, though I don't like it.

jakkdl

I'm more positive on enabling this than a5rocks or teamspen, I think there's pretty good value from catching future PR's from adding unneeded Anys. Changing them after the fact with somebody manually searching for Any, or even having to complain in reviews, will be far less efficient.
If some file in _core gets really messy from having to type: ignore everything then we can disable it specifically for that file in the config.

Generally I also think that it's better practice to use object and then type: ignore[...] where the problem actually arises - that gives the reader a better understanding of where the type system stops giving any help and/or why we need to override it, rather than saying "we don't typecheck this parameter, go read the rest of the function to figure out why". But yeah if it surfaces several error messages then silencing it once is the way to go

A bit irritating that there's no type: ignore[no-explicit-any], could probably request that be added upstream.

src/trio/_core/_concat_tb.py

src/trio/_core/_io_windows.py

src/trio/_core/_run.py

src/trio/_core/_tests/test_run.py

jakkdl · 2024-10-28T11:56:21Z

src/trio/_core/_thread_cache.py

+    __slots__ = ("_default_name", "_job", "_thread", "_thread_cache", "_worker_lock")
+


lol. Why are there slots changes in this PR?

Yea, after the fact I was considering moving that to another pull request, but basically while handling issues I noticed this didn't have slots, so I added it.

jakkdl · 2024-10-28T11:57:15Z

src/trio/_core/_thread_cache.py

 class ThreadCache:
+    __slots__ = ("_idle_workers",)
+
    def __init__(self) -> None:
-        self._idle_workers: dict[WorkerThread[Any], None] = {}
+        # Explicit "Any" not allowed
+        self._idle_workers: dict[WorkerThread[Any], None] = {}  # type: ignore[misc]


prime opportunity for using attrs

jakkdl · 2024-10-28T11:57:31Z

src/trio/_core/_traps.py

-from typing import TYPE_CHECKING, Any, Callable, NoReturn
+from collections.abc import Awaitable
+
+# Jedi gets mad in test_static_tool_sees_class_members if we use collections Callable


huh, something to report upstream?

src/trio/_core/_traps.py

src/trio/_socket.py

Co-authored-by: John Litborn <11260241+jakkdl@users.noreply.github.com>

jakkdl

phew, big review!

src/trio/_core/_io_windows.py

src/trio/_core/_ki.py

jakkdl · 2024-10-29T10:13:23Z

src/trio/_core/_run.py

@@ -103,8 +103,7 @@ class _NoStatus(metaclass=NoPublicConstructor):

 # Decorator to mark methods public. This does nothing by itself, but
 # trio/_tools/gen_exports.py looks for it.
-# Explicit "Any" is not allowed
-def _public(fn: FnT) -> FnT:  # type: ignore[misc]
+def _public(fn: Callable[PS, RetT]) -> Callable[PS, RetT]:


I was considering changing to def _public(fn: T) -> T: because that's all it really does, and I think this cements that that might be a better solution.

well, the comment specifies that it's only used "to mark methods public" so using FnT is a way of enforcing that. But it's probably fine to do T->T

src/trio/_core/_run.py

src/trio/_threads.py

src/trio/_util.py

Co-authored-by: John Litborn <11260241+jakkdl@users.noreply.github.com>

jakkdl

Great!

CoolCat467 · 2024-11-01T01:32:39Z

@A5rocks ?

A5rocks · 2024-11-01T08:40:18Z

I'm neutral on this and looking at some of the changes this seems fine. It's a bit strange that mypy marks usage of typevars with Any bounds as usage of Any but oh well.

CoolCat467 · 2024-11-01T14:17:30Z

Will merge this in a day or two unless anyone has any other comments

Enable mypy's disallow_any_explicit flag.

6e91b75

agreed. disable ANN401 for now and enable `disallow_any_explicit` in a separate PR. Possibly same for `disallow_untyped_defs`. _Originally posted by @jakkdl in python-trio#3098 (comment)

CoolCat467 added dependencies Pull requests that update a dependency file skip newsfragment Newsfragment is not required labels Oct 27, 2024

CoolCat467 requested review from TeamSpen210, jakkdl and A5rocks October 27, 2024 06:17

CoolCat467 added 2 commits October 27, 2024 01:37

Fix various things after reviewing all changes

feeccf1

Do not use faster mypy cache on pypy

569bc37

CoolCat467 added 2 commits October 27, 2024 16:35

Merge remote-tracking branch 'origin/main' into enable-disallow_any_e…

deb1c13

…xplicit

Try to get rid of more Anys

521c1b7

TeamSpen210 reviewed Oct 27, 2024

View reviewed changes

CoolCat467 added 6 commits October 28, 2024 00:43

Get rid of a bunch more Anys and resolve mypy issues

55964ad

Match other TypeVarTuple usage

e888514

Mark variable as nonlocal

30db0d8

Cast asyncio run in loop to be InHost

7707361

Handle case where names does not exist in node for some reason

e99b69f

Hopefully fix jedi issue

aee4a77

Check the hashes are the same

e157589

A5rocks added 3 commits October 28, 2024 03:52

Get the hash glob makes

62f89fa

Fix inputs to glob.hashFiles

ca48ef6

Remove testing hashes

28b4abc

jakkdl requested changes Oct 28, 2024

View reviewed changes

CoolCat467 and others added 2 commits October 28, 2024 11:07

Apply requested changes from code review

a1e4916

Co-authored-by: John Litborn <11260241+jakkdl@users.noreply.github.com>

Code review suggestions

c3c0a28

Co-authored-by: John Litborn <11260241+jakkdl@users.noreply.github.com>

CoolCat467 force-pushed the enable-disallow_any_explicit branch from a144f98 to c3c0a28 Compare October 28, 2024 16:11

CoolCat467 added 4 commits October 28, 2024 12:40

Fix type issue

63353f4

Fix cffi type issues again

69e60a5

Merge branch 'main' into enable-disallow_any_explicit

25ecf1e

Use correct CData

2bc0da3

CoolCat467 removed the skip newsfragment Newsfragment is not required label Oct 28, 2024

Fix cast again

0f999b8

jakkdl requested changes Oct 29, 2024

View reviewed changes

CoolCat467 and others added 11 commits October 30, 2024 00:14

Clarify comments and get rid of more Anys

50adccb

Co-authored-by: John Litborn <11260241+jakkdl@users.noreply.github.com>

Update src/trio/_tests/test_socket.py

3a320c3

Co-authored-by: John Litborn <11260241+jakkdl@users.noreply.github.com>

Apply suggestions from code review

03f1c4e

Co-authored-by: John Litborn <11260241+jakkdl@users.noreply.github.com>

Apply suggestions from code review

33f1caa

Co-authored-by: John Litborn <11260241+jakkdl@users.noreply.github.com>

Update src/trio/_threads.py

854c5cd

Co-authored-by: John Litborn <11260241+jakkdl@users.noreply.github.com>

Improve type annotations

bcd3f54

Co-authored-by: John Litborn <11260241+jakkdl@users.noreply.github.com>

Merge branch 'main' into enable-disallow_any_explicit

7e719a8

Forgot type and fix more type issues

ee0322a

Add typing to orig_getaddrinfo

9005f98

Add full typing for _responses and record

c480449

Fix type issues

ba24f74

CoolCat467 requested review from jakkdl and TeamSpen210 October 30, 2024 18:34

jakkdl approved these changes Oct 31, 2024

View reviewed changes

CoolCat467 merged commit c7801ae into python-trio:main Nov 3, 2024
39 checks passed

CoolCat467 deleted the enable-disallow_any_explicit branch November 3, 2024 04:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable mypy's `disallow_any_explicit` flag. #3121

Enable mypy's `disallow_any_explicit` flag. #3121

CoolCat467 commented Oct 27, 2024 •

edited

Loading

codecov bot commented Oct 27, 2024 •

edited

Loading

A5rocks commented Oct 27, 2024

TeamSpen210 commented Oct 27, 2024

TeamSpen210 Oct 27, 2024

CoolCat467 Oct 28, 2024

CoolCat467 Oct 28, 2024

jakkdl Oct 29, 2024

CoolCat467 commented Oct 28, 2024

A5rocks commented Oct 28, 2024

A5rocks commented Oct 28, 2024 •

edited

Loading

jakkdl left a comment •

edited

Loading

jakkdl Oct 28, 2024

CoolCat467 Oct 28, 2024

jakkdl Oct 28, 2024

jakkdl Oct 28, 2024

jakkdl left a comment

jakkdl Oct 29, 2024

jakkdl left a comment

CoolCat467 commented Nov 1, 2024

A5rocks commented Nov 1, 2024

CoolCat467 commented Nov 1, 2024

		__slots__ = ("_default_name", "_job", "_thread", "_thread_cache", "_worker_lock")

Enable mypy's disallow_any_explicit flag. #3121

Enable mypy's disallow_any_explicit flag. #3121

Conversation

CoolCat467 commented Oct 27, 2024 • edited Loading

codecov bot commented Oct 27, 2024 • edited Loading

Codecov Report

A5rocks commented Oct 27, 2024

TeamSpen210 commented Oct 27, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CoolCat467 commented Oct 28, 2024

A5rocks commented Oct 28, 2024

A5rocks commented Oct 28, 2024 • edited Loading

jakkdl left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jakkdl left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jakkdl left a comment

Choose a reason for hiding this comment

CoolCat467 commented Nov 1, 2024

A5rocks commented Nov 1, 2024

CoolCat467 commented Nov 1, 2024

Enable mypy's `disallow_any_explicit` flag. #3121

Enable mypy's `disallow_any_explicit` flag. #3121

CoolCat467 commented Oct 27, 2024 •

edited

Loading

codecov bot commented Oct 27, 2024 •

edited

Loading

A5rocks commented Oct 28, 2024 •

edited

Loading

jakkdl left a comment •

edited

Loading