Repository navigation
gh-108518: Make concurrent.futures.Executor.map() consistent with built-in map() - #109497
serhiy-storchaka merged 11 commits into
Conversation
Sorry, something went wrong.
serhiy-storchaka
left a comment
There was a problem hiding this comment.
Good code, but slightly over-engineered. I think that it will be better after simplification.
And there is a regression (removing the close() method).
Sorry, something went wrong.
| __class_getitem__ = classmethod(types.GenericAlias) | ||
|
|
||
|
|
||
| class _FutureResult(object): |
There was a problem hiding this comment.
It looks over-engineered. Why not simply use a result-exception tuple?
Sorry, something went wrong.
| @classmethod | ||
| def from_generator(cls, gen): | ||
| return cls(gen) |
There was a problem hiding this comment.
It is redundant. You can simply use constructor.
Sorry, something went wrong.
| return False | ||
|
|
||
|
|
||
| class _MapResultIterator(object): |
There was a problem hiding this comment.
(object) is redundant.
Current iterator has the close() method, calling which cancels all futures. It is a useful feature. Please add a close() method in a new class. I do not think that it is worth to implement send() and throw(), they are not so useful and are rather an implementation detail.
Sorry, something went wrong.
| ) as gen: | ||
| with self.assertRaises(TimeoutError): | ||
| next(gen) | ||
| iterator = pool.map(log_n_wait, ["second"], timeout=0) |
There was a problem hiding this comment.
This code should continue to work without changes.
Sorry, something went wrong.
serhiy-storchaka
left a comment
There was a problem hiding this comment.
I resolved conflicts, fixed some errors, simplified code, added the close method, added docs and more tests. TimeoutError immediately cancels all calls.
Now this PR LGTM.
@pitrou, @brianquinlan, @vstinner, @gpshead, could anybody of your please take a look at this if you have a time?
Sorry, something went wrong.
|
This PR is stale because it has been open for 30 days with no activity. |
Sorry, something went wrong.
# Conflicts: # Lib/concurrent/futures/_base.py # Lib/concurrent/futures/process.py # Lib/test/test_concurrent_futures/executor.py
Documentation build overview3 files changed ± library/concurrent.futures.html ± whatsnew/3.16.html ± whatsnew/changelog.html |
Sorry, something went wrong.
…rror (pythonGH-109497) If a call raises an exception, the remaining calls are no longer cancelled and iteration can be continued. Use the close() method of the returned iterator to cancel them. Co-authored-by: Meng Xiangzhuo <aumo@foxmail.com> Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
The current behavior of concurrent.futures.Executor.map() is not consistent with documentation:
When a next call raises an Exception, you can not get any subsequent Future result, even if the computation has completed and returns a valid result.
If it is expected or don't consider to make changes to this behavior, I'd like to make a PR to update the relevant docs.