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

Update to mypy 1.15 #3201

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Update to mypy 1.15 #3201

wants to merge 5 commits into from

Conversation

tjstum
Copy link
Member

@tjstum tjstum commented Feb 5, 2025

In python/typeshed#13372, the type of the stdlib's socket.getaddrinfo was updated to add a union member for cases where Python was compiled with --disable-ipv6. This causes custom resolvers that use the stdlib function to fail mypy

This is probably going to (mypy) break everyone's custom HostnameResolver, since the parent type is changing

In python/typeshed#13372, the type of the
stdlib's `socket.getaddrinfo` was updated to add a union member for
cases where Python was compiled with `--disable-ipv6`. This causes
custom resolvers that use the stdlib function to fail mypy

This is probably going to (mypy) break everyone's custom
`HostnameResolver`, since the parent type is changing
Copy link

codecov bot commented Feb 5, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.95744%. Comparing base (8a7674c) to head (8209ec1).
Report is 2 commits behind head on main.

Additional details and impacted files
@@                 Coverage Diff                  @@
##                 main       #3201         +/-   ##
====================================================
- Coverage   100.00000%   99.95744%   -0.04256%     
====================================================
  Files             124         124                 
  Lines           18792       18796          +4     
  Branches         1268        1268                 
====================================================
- Hits            18792       18788          -4     
- Misses              0           8          +8     
Files with missing lines Coverage Δ
src/trio/_abc.py 100.00000% <ø> (ø)
src/trio/_core/_entry_queue.py 100.00000% <100.00000%> (ø)
src/trio/_core/_instrumentation.py 100.00000% <100.00000%> (ø)
src/trio/_core/_run.py 100.00000% <100.00000%> (ø)
src/trio/_core/_tests/test_asyncgen.py 100.00000% <100.00000%> (ø)
src/trio/_core/_tests/test_ki.py 100.00000% <100.00000%> (ø)
src/trio/_core/_tests/test_run.py 100.00000% <100.00000%> (ø)
src/trio/_core/_thread_cache.py 100.00000% <100.00000%> (ø)
src/trio/_core/_traps.py 100.00000% <100.00000%> (ø)
src/trio/_file_io.py 100.00000% <ø> (ø)
... and 15 more

... and 1 file with indirect coverage changes

@A5rocks
Copy link
Contributor

A5rocks commented Feb 5, 2025

I guess this PR should be more generally "update to mypy 1.15"...

And major typing changes are fine, type hints are optional/not at runtime anyways.

@CoolCat467 CoolCat467 added dependencies Pull requests that update a dependency file typing Adding static types to trio's interface labels Feb 5, 2025
@tjstum
Copy link
Member Author

tjstum commented Feb 5, 2025

OK, I'll take a look at the other typing changes needed for 1.15 as well

@jakkdl
Copy link
Member

jakkdl commented Feb 6, 2025

looks like we need to replace a lot of type: ignore[misc] with type: ignore[explicit-any]

@tjstum tjstum changed the title Update typing for HostnameResolver Update to mypy 1.15 Feb 7, 2025
Copy link
Contributor

@A5rocks A5rocks left a comment

Choose a reason for hiding this comment

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

I actually just was looking at doing this and thinking through what I would prefer, but I actually prefer the choices you made here (e.g. pushing up the CoroutineType change rather than littering code with assert isinstance(task.coro, CoroutineType))

A couple comments, but most are very nitpicky so feel free to ignore.

src/trio/_core/_run.py Outdated Show resolved Hide resolved
src/trio/_core/_tests/test_asyncgen.py Outdated Show resolved Hide resolved
src/trio/_file_io.py Outdated Show resolved Hide resolved
src/trio/_tests/test_socket.py Outdated Show resolved Hide resolved
src/trio/_tests/test_socket.py Outdated Show resolved Hide resolved
src/trio/_threads.py Outdated Show resolved Hide resolved
Comment on lines +37 to +38
additional_dependencies:
- tomli
Copy link
Member Author

Choose a reason for hiding this comment

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

Without this change, pre-commit was not picking up the config settings in pyproject.toml. I'm curious how others haven't been running into this.

Copy link
Contributor

Choose a reason for hiding this comment

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

Well I have Python 3.13 locally and so maybe it uses tomllib (3.11+) instead? Wouldn't be surprised if it's just a variant of that for everyone ... or they just don't run pre-commit.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I'm using 3.10 in the venv I do trio work in.

Copy link
Member

Choose a reason for hiding this comment

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

I've been meaning to add this for quite some time, thanks for catching this!

src/trio/_core/_run.py Outdated Show resolved Hide resolved
src/trio/_core/_tests/test_asyncgen.py Outdated Show resolved Hide resolved
src/trio/_tests/test_socket.py Outdated Show resolved Hide resolved
src/trio/_threads.py Outdated Show resolved Hide resolved
Copy link

@Hnasar Hnasar left a comment

Choose a reason for hiding this comment

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

Superb!

Comment on lines +212 to +213
class RunSync(Generic[RetT]): # type: ignore[explicit-any]
fn: Callable[..., RetT] # type: ignore[explicit-any]
Copy link

Choose a reason for hiding this comment

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

can you use Unpack[Ts] here too?

match that of the stdlib `socket.getaddrinfo`, which was updated in mypy 1.15 (via
a typeshed update) to include the possibility of ``tuple[int, bytes]`` for the
``sockaddr`` field of the result. This happens in situations where Python was compiled
with ``--disable-ipv6``.
Copy link

Choose a reason for hiding this comment

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

It's probably worth calling out the improved typing around to_thread/from_thread functions too.

Copy link
Contributor

@TeamSpen210 TeamSpen210 left a comment

Choose a reason for hiding this comment

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

Not sure if any @_public users have overloads, but if so:



# 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: F) -> F: # type: ignore[misc]
def _public(fn: Callable[P, T]) -> Callable[P, T]:
Copy link
Contributor

Choose a reason for hiding this comment

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

This should not be ParamSpec. There's a subtle difference between this and a callable-bound TypeVar - ParamSpec can't capture an overload since the return type might change, but the typevar form preserves it, so it would be passed through unchanged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file typing Adding static types to trio's interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants