8000 Base concurrent.futures.Executor on ContextManager by joshstaiger · Pull Request #1711 · python/typeshed · GitHub
[go: up one dir, main page]
Skip to content

Base concurrent.futures.Executor on ContextManager #1711

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.

Sign up for GitHub

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 5 commits into from
Nov 4, 2017
Merged

Base concurrent.futures.Executor on ContextManager #1711

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
99cc86c
Base concurrent.futures.Executor on ContextManager
joshstaiger Nov 3, 2017
260c1ba
Fix flake8 complaint.
joshstaiger Nov 3, 2017
cacaa2c
Revert "Fix flake8 complaint."
joshstaiger Nov 4, 2017
89ec503
Revert "Base concurrent.futures.Executor on ContextManager"
joshstaiger Nov 4, 2017
0ccc3f0
Make Executor.__enter__ self and return types match.
joshstaiger Nov 4, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 19 additions & 6 deletions stdlib/3/concurrent/futures/_base.pyi
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
from typing import TypeVar, Generic, Any, Iterable, Iterator, Callable, Tuple, Optional, Set, NamedTuple

import sys

FIRST_COMPLETED = ... # type: str
FIRST_EXCEPTION = ... # type: str
ALL_COMPLETED = ... # type: str
Expand Down Expand Up @@ -31,12 +33,23 @@ class Future(Generic[_T]):
def set_result(self, result: _T) -> None: ...
def set_exception(self, exception: Optional[BaseException]) -> None: ...

class Executor:
def submit(self, fn: Callable[..., _T], *args: Any, **kwargs: Any) -> Future[_T]: ...
def map(self, func: Callable[..., _T], *iterables: Iterable[Any], timeout: Optional[float] = ..., chunksize: int = ...) -> Iterator[_T]: ...
def shutdown(self, wait: bool = ...) -> None: ...
def __enter__(self) -> Executor: ...
Copy link
Member

Choose a reason for hiding this comment

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

Why can't you achieve what you want with the simpler solution of typing __enter__ as def __enter__(self: _T) -> _T: ..., where _T is a TypeVar bound to Executor? That would make the rest of this PR unnecessary and remove the version_info checks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That would work, too.

I didn't do it that way because of the note here about generic selfs being experimental and not fully implemented.

But if you think that's the way forward I can rework it like that.

Copy link
Member

Choose a reason for hiding this comment

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

But self is not generic here, is it? Executor itself is not a generic class.

Copy link
Contributor Author
@joshstaiger joshstaiger Nov 4, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

I just meant "generic self" to mean that self has a TypeVar type (which I believe is what the note is describing).

According to the note, I don't think it matters if the class (Executor) that gets bound to the TypeVar is itself a 'generic class'.

See the example, none of the Shape, Circle, or Square are generic classes either.

Copy link
Member

Choose a reason for hiding this comment

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

My understanding is that self-types are fairly stable for non-generic classes (like the ones we're using here). The troubles arise when the class we're using a self type for is itself generic: see python/mypy#2354, python/mypy#3363, python/mypy#3631.

Copy link
Member

Choose a reason for hiding this comment

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

In any case, self types are already widely used in typeshed for non-generic classes (grep finds about 130 usages). I'm confident that using them here too is safe.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, then! I'll rework this that way when I get a moment.

def __exit__(self, exc_type: Any, exc_val: Any, exc_tb: Any) -> bool: ...

if sys.version_info >= (3, 6):
from typing import ContextManager
_S = TypeVar('_S')

class Executor(ContextManager[_S]):
def submit(self, fn: Callable[..., _T], *args: Any, **kwargs: Any) -> Future[_T]: ...
def map(self, func: Callable[..., _T], *iterables: Iterable[Any], timeout: Optional[float] = ..., chunksize: int = ...) -> Iterator[_T]: ...
def shutdown(self, wait: bool = ...) -> None: ...

else:
class Executor:
def submit(self, fn: Callable[..., _T], *args: Any, **kwargs: Any) -> Future[_T]: ...
def map(self, func: Callable[..., _T], *iterables: Iterable[Any], timeout: Optional[float] = ..., chunksize: int = ...) -> Iterator[_T]: ...
def shutdown(self, wait: bool = ...) -> None: ...
def __enter__(self) -> Executor: ...
def __exit__(self, exc_type: Any, exc_val: Any, exc_tb: Any) -> bool: ...

def as_completed(fs: Iterable[Future[_T]], timeout: Optional[float] = ...) -> Iterator[Future[_T]]: ...

Expand Down
12 changes: 10 additions & 2 deletions stdlib/3/concurrent/futures/process.pyi
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,17 @@
from typing import Optional, Any

import sys

from ._base import Future, Executor

EXTRA_QUEUED_CALLS = ... # type: Any

class BrokenProcessPool(RuntimeError): ...

class ProcessPoolExecutor(Executor):
def __init__(self, max_workers: Optional[int] = ...) -> None: ...
if sys.version_info >= (3, 6):
class ProcessPoolExecutor(Executor[ProcessPoolExecutor]):
def __init__(self, max_workers: Optional[int] = ...) -> None: ...

else:
class ProcessPoolExecutor(Executor):
def __init__(self, max_workers: Optional[int] = ...) -> None: ...
11 changes: 6 additions & 5 deletions stdlib/3/concurrent/futures/thread.pyi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@ from typing import Optional
from ._base import Executor, Future
import sys

class ThreadPoolExecutor(Executor):
if sys.version_info >= (3, 6):
def __init__(self, max_workers: Optional[int] = ...,
thread_name_prefix: str = ...) -> None: ...
else:
if sys.version_info >= (3, 6):
class ThreadPoolExecutor(Executor[ThreadPoolExecutor]):
def __init__(self, max_workers: Optional[int] = ..., thread_name_prefix: str = ...) -> None: ...

else:
class ThreadPoolExecutor(Executor):
def __init__(self, max_workers: Optional[int] = ...) -> None: ...
0