Make multiprocessing.Queue a subclass of queue.Queue #1525
Conversation
| @@ -93,10 +94,10 @@ class Process(): | |||
| def is_alive(self) -> bool: ... | |||
| def join(self, timeout: Optional[float] = ...) -> None: ... | |||
|
|
|||
| class Queue(): | |||
| class Queue(queue.Queue): | |||
There was a problem hiding this comment.
So this implicitly makes the base class queue.Queue[Any]. I wonder if it wouldn't be better to make this Queue class generic as well, so one can instantiate a mp.Queue[int], say.
There was a problem hiding this comment.
I'm not that familiar with the mypy types yet, so I'm not sure if I'm doing this correctly, but changing it to a Generic[_T] like queue.Queue gives me error: Need type annotation for variable when I do q = mp.Queue()
|
Hm, that's also the case when instantiating |
bcl
force-pushed
the
master-multiprocessing.Queue
branch
from
ce4f291 to
9488b31
Compare
|
Ah! Right, that makes sense. |
bcl
force-pushed
the
master-multiprocessing.Queue
branch
from
9488b31 to
27ee043
Compare
|
I'm not sure what's up with that travis failure. When I run the tests here they all pass: |
|
I've restarted the test job. I believe it's an intermittent Travis failure: python/mypy#3543. |
| def get(self, block: bool = ..., timeout: float = ...) -> Any: ... | ||
| def put(self, item: Any, block: bool = ..., timeout: float = ...) -> None: ... | ||
| def get(self, block: bool = ..., timeout: Optional[float] = ...) -> Any: ... | ||
| def put(self, item: Any, block: bool = ..., timeout: Optional[float] = ...) -> None: ... |
There was a problem hiding this comment.
The two Anys in get() and put() should be replaced with _T, to match the same methods in queue.Queue.
Also change multiprocessing.Queue's put and get timeout arguments to allow None. This fixes a problem with logging.handlers.QueueHandler and QueueListener not accepting a multiprocessing.Queue as the queue argument. Declaring the Queue now needs to note what it will be used for. eg. q = multiprocessing.Queue() # type: multiprocessing.Queue[List[Any]]
bcl
force-pushed
the
master-multiprocessing.Queue
branch
from
27ee043 to
16843f6
Compare
|
Oops, sorry about that. Also changed Any in put_nowait and get_nowait to _T |
|
Thanks! I am still a little worried that this may break code that's using this without an annotation; we'll see how it goes in practice. |
|
Oh, PS. Next time please don't squash your local commits -- this makes it harder to track a code review, and it's not needed, since we already squash when we merge into master. |
|
Will do. Thanks! |
Also change multiprocessing.Queue's put and get timeout arguments to
allow None.
This fixes a problem with logging.handlers.QueueHandler and
QueueListener not accepting a multiprocessing.Queue as the queue
argument.