Skip to content

attempt to add typing to _create #3260

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

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Conversation

jakkdl
Copy link
Member

@jakkdl jakkdl commented May 1, 2025

context: #3256 (comment)

@A5rocks

remaining errors:

src/trio/_core/_local.py:27: error: Argument 1 to "_create" of "NoPublicConstructor" has incompatible type "RunVar[T]"; expected "Never"  [arg-type]
src/trio/_channel.py:265: error: Argument 1 to "_create" of "NoPublicConstructor" has incompatible type "MemoryChannelState[SendType]"; expected "Never"  [arg-type]
src/trio/_channel.py:417: error: Argument 1 to "_create" of "NoPublicConstructor" has incompatible type "MemoryChannelState[ReceiveType]"; expected "Never"  [arg-type]

my hypothesis for these is that they all inherit from some other class which doesn't have this parameter, and _create sees the super class. idk how to fix that

src/trio/_core/_run.py:1907: error: Argument "coro" to "_create" of "NoPublicConstructor" has incompatible type "Coroutine[object, Never, object]"; expected "CoroutineType[Any, Outcome[object], Any]"  [arg-type]

idk who's in the wrong here

src/trio/_subprocess.py:430: error: Argument 1 to "_create" of "NoPublicConstructor" has incompatible type "Popen[str]"; expected "Popen[bytes]"  [arg-type]

this might be an actual bug? https://github.com/python/typeshed/blob/eec809d049d10a5ae9b88780eab15fe36a9768d7/stdlib/subprocess.pyi#L1464 for reference

@jakkdl jakkdl added the skip newsfragment Newsfragment is not required label May 1, 2025
Copy link

codecov bot commented May 1, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00000%. Comparing base (9c909fa) to head (b08cc92).

Additional details and impacted files
@@               Coverage Diff               @@
##                 main        #3260   +/-   ##
===============================================
  Coverage   100.00000%   100.00000%           
===============================================
  Files             124          124           
  Lines           19047        19047           
  Branches         1287         1287           
===============================================
  Hits            19047        19047           
Files with missing lines Coverage Δ
src/trio/_util.py 100.00000% <100.00000%> (ø)
src/trio/testing/_fake_net.py 100.00000% <100.00000%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

For the _create calls, I know Never often appears if mypy failed to solve the typevars, so could be an inference failure. Might be good to add reveal_type(TheClass._create) calls so we can see what mypy assumes it is.

return super().__call__(*args, **kwargs) # type: ignore
def _create(cls: Callable[P, T], *args: P.args, **kwargs: P.kwargs) -> T:
# misc = unsupported argument 2 for "super" (??)
return super().__call__(*args, **kwargs) # type: ignore[misc,no-any-return]
Copy link
Contributor

Choose a reason for hiding this comment

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

What's going on here is the cls: Callable makes MyPy forget that it should require a class - NoPublicConstructor._create(lambda: 0) should function. It is useful to be able to widen self-types like that, but it means the super() call fails. I guess Mypy type-checks the 2-arg form of super() automatically?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, it's not wrong surprisingly:

>>> class Xs:
...     def f(self) -> None:
...         pass
...
>>> class X(Xs):
...     def f(self: object) -> None:
...         super().f()
...         return
...
>>> Xs().f()
>>> X().f()
>>> X.f(X())
>>> X.f(5)
Traceback (most recent call last):
  File "<python-input-7>", line 1, in <module>
    X.f(5)
    ~~~^^^
  File "<python-input-3>", line 3, in f
    super().f()
    ^^^^^^^^^
TypeError: super(type, obj): obj (instance of int) is not an instance or subtype of type (X).

@CoolCat467 CoolCat467 added the typing Adding static types to trio's interface label May 1, 2025
@jakkdl
Copy link
Member Author

jakkdl commented May 2, 2025

For the _create calls, I know Never often appears if mypy failed to solve the typevars, so could be an inference failure. Might be good to add reveal_type(TheClass._create) calls so we can see what mypy assumes it is.

reveal_type(cls._create) in class RunVarToken(Generic[T], metaclass=NoPublicConstructor)

src/trio/_core/_local.py:27: note: Revealed type is "def (*args: Never, **kwargs: Never) -> trio._core._local.RunVarToken[T`1]"

reveal_type(MemorySendChannel._create)

src/trio/_channel.py:265: note: Revealed type is "def (*args: Never, **kwargs: Never) -> Never"

@jakkdl
Copy link
Member Author

jakkdl commented May 2, 2025

also if anybody wants to mess around with this PR you are very welcome, I'm not gonna put a lot of effort into it

@A5rocks
Copy link
Contributor

A5rocks commented May 3, 2025

Looks like this approach doesn't handle type vars (I think that's what you're seeing):

import dataclasses
from typing import Protocol, TypeVar, ParamSpec, Generic, Callable

T = TypeVar("T", covariant=True)
P = ParamSpec("P")

class Meta(type):
    def __call__(cls, *args: object, **kwargs: object) -> None:
        raise TypeError(
            f"{cls.__module__}.{cls.__qualname__} has no public constructor",
        )

    def _create(cls: Callable[P, T], *args: P.args, **kwargs: P.kwargs) -> type[T]:
        return super().__call__(*args, **kwargs)  # type: ignore

@dataclasses.dataclass
class X(Generic[T], metaclass=Meta):
    x: T


reveal_type(X._create)  # N: Revealed type is "def (*args: Never, **kwargs: Never) -> type[Never]"

Surprisingly now that I see it minimized like that, I think I know exactly the bug: python/mypy#18400. I haven't checked my solution PR to make sure it has the same cause though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip newsfragment Newsfragment is not required typing Adding static types to trio's interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants