Skip to content

Add public missing asyncio stubs for windows and proactor files #3234

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

Merged
merged 9 commits into from
Oct 15, 2019

Conversation

CraftSpider
Copy link
Contributor

@CraftSpider CraftSpider commented Sep 16, 2019

More work towards #2313.

Adds in all the windows_*.py and proactor_*.py files. There are mypy test failures, but I don't want to add stubs for those methods without discussion because the real implementation doesn't actually have them. create_unix_* are not actual methods on windows-specific asyncio loops.

…any necessary private return/argument types.
@JelleZijlstra
Copy link
Member

Sorry for the delay in reviewing this PR! The CI failures obviously need to be dealt with. The best option is probably to add these methods to the stub with a comment that they don't really exist at runtime.

@CraftSpider
Copy link
Contributor Author

Alright, after a merge and fixes, build is now passing. Added the comment about runtime status of the methods in BaseProactorEventLoop

Copy link
Member

@JelleZijlstra JelleZijlstra left a comment

Choose a reason for hiding this comment

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

Sorry for another delay! I have some minor comments.


class _ProactorBasePipeTransport(transports._FlowControlMixin, transports.BaseTransport):

def __init__(self, loop: events.AbstractEventLoop, sock: socket, protocol: streams.StreamReaderProtocol, waiter: futures.Future[Any] = ..., extra: Mapping[Any, Any] = ..., server: events.AbstractServer = ...) -> None: ...
Copy link
Member

Choose a reason for hiding this comment

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

waiter, extra, and server should be Optional (also in the other classes below).

def __init__(self, loop: events.AbstractEventLoop, sock: socket, protocol: streams.StreamReaderProtocol, waiter: futures.Future[Any] = ..., extra: Mapping[Any, Any] = ..., server: events.AbstractServer = ...) -> None: ...

class _ProactorDuplexPipeTransport(_ProactorReadPipeTransport, _ProactorBaseWritePipeTransport, transports.Transport): ...

Copy link
Member

Choose a reason for hiding this comment

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

Why isn't _ProactorSocketTransport included?

@@ -43,3 +44,7 @@ class SubprocessTransport(BaseTransport):
def send_signal(self, signal: int) -> int: ...
def terminate(self) -> None: ...
def kill(self) -> None: ...

class _FlowControlMixin(Transport):
def __init__(self, extra: Mapping[Any, Any] = ..., loop: AbstractEventLoop = ...) -> None: ...
Copy link
Member

Choose a reason for hiding this comment

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

Both of the arguments are Optional

import socket
from . import proactor_events, events, futures, windows_utils, selector_events

NULL: int = ...
Copy link
Member

Choose a reason for hiding this comment

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

The = ... can be omitted.


class ProactorEventLoop(proactor_events.BaseProactorEventLoop):

def __init__(self, proactor: IocpProactor = ...) -> None: ...
Copy link
Member

Choose a reason for hiding this comment

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

Optional

class ProactorEventLoop(proactor_events.BaseProactorEventLoop):

def __init__(self, proactor: IocpProactor = ...) -> None: ...
async def create_pipe_connection(self, protocol_factory: Callable[[], events.AbstractEventLoopPolicy], address: str) -> Tuple[proactor_events._ProactorDuplexPipeTransport, events.AbstractEventLoopPolicy]: ...
Copy link
Member

Choose a reason for hiding this comment

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

AbstractEventLoopPolicy seems wrong. Surely it should be some sort of Protocol?

Copy link
Contributor Author

@CraftSpider CraftSpider Oct 15, 2019

Choose a reason for hiding this comment

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

Pretty sure you're right, it actually expects a StreamReaderProtocol from what I can tell. Not sure why I put it as AbstractEventLoopPolicy


class IocpProactor:

def __init__(self, concurrency: int = ...): ...
Copy link
Member

Choose a reason for hiding this comment

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

Missing -> None

def __repr__(self) -> str: ...
def __del__(self) -> None: ...
def set_loop(self, loop: events.AbstractEventLoop) -> None: ...
def select(self, timeout: int = ...) -> List[futures.Future[Any]]: ...
Copy link
Member

Choose a reason for hiding this comment

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

timeout is Optional

def __repr__(self) -> str: ...
def __del__(self) -> None: ...
def __enter__(self) -> PipeHandle: ...
def __exit__(self, t: type, v: BaseException, tb: TracebackType) -> None: ...
Copy link
Member

Choose a reason for hiding this comment

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

Strictly all the arguments should be Optional.

Copy link
Member

@JelleZijlstra JelleZijlstra left a comment

Choose a reason for hiding this comment

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

Thanks!

@JelleZijlstra JelleZijlstra merged commit dfe6862 into python:master Oct 15, 2019
@CraftSpider CraftSpider deleted the asyncio-stubs branch October 16, 2019 00:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants