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
bpo-27873: Update docstring for multiprocessing.Pool.map #17367
Conversation
|
Hello, and thanks for your contribution! I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA). CLA MissingOur records indicate the following people have not signed the CLA: For legal reasons we need all the people listed to sign the CLA before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue. If you have recently signed the CLA, please wait at least one business day You can check yourself to see if the CLA has been received. Thanks again for the contribution, we look forward to reviewing it! |
|
Thanks for the contribution to CPython project :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@berkerpeksag
Dear core developers,
@aisk is the first time contributor to this project.
This PR looks good to me except the CLA is not done yet.
Please take a look @aisk PR kindly.
Thank you for understanding :)
|
@corona10 Hi thanks for your welcome! Happy to see you again! The CLA is signed, and I think it's taking time to complete? The bot said it's taking a work day usually. |
Yes, it is expected to take some time. Let's wait until then. :) If you need my help please ping me any time Thanks for your contribution! |
|
😃 Thank you again! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR @aisk and welcome!
I approve the general goal of the PR, but I would recommend a slight change in wording:
A parallel equivalent of the built-in function :func:`map`. It blocks
until the result is ready.
Unlike :func:`map`, this method supports only one *iterable*
argument. For functions that require multiple arguments, see
:meth:`starmap`.
(Note: adjusted the word wrapping because it exceeded 80 chars per line with the changes I recommended)
Co-authored-by: naught101 .
b157e8f
to
ca2630d
Compare
|
Updated, and the CLA check is passed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for making the recommended changes @aisk, LGTM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
|
Dear @rhettinger This PR is the @aisk 's first time contributing to this project. |
|
Sorry, I'm going to recommend rejecting this edit as being unnecesary. The argument signature already shows that only one iterable is accepted. AFAICT, no one has ever been confused by the current docs. There has been interest in changing the signature to accommodate multiple inputs, but that is a different matter than whether the current docs are accurate. |
|
@rhettinger Hi I'm fine with this, and have another question. If we change the signature to allow it accept multiple iterable as input, how to compat with old codes? Some developer could call this with chunksize without keyword argument like Can we just check the chunksize's type and make it compatible? or mark it as a breaking change? |
While there's already mention of
Perhaps the PR could be edited to leave the existing "supports only one iterable" part intact, and modify it only slightly to mention Also, it might be helpful to obtain additional input on this from @pitrou, since he's an active expert/maintainer for the multiprocessing module. |
|
I'd be in favour of @aeros' proposed change. Feel free to open a new PR :-) |
|
@aeros ok, I'll try it later this day, thanks! |
) Update docstring for `multiprocessing.Pool.map` to mention `pool.starmap()`. Prev PR: python#17367 @aeros https://bugs.python.org/issue27873 (cherry picked from commit eb48a45) Co-authored-by: An Long <aisk@users.noreply.github.com>
Update docstring for `multiprocessing.Pool.map` to mention `pool.starmap()`. Prev PR: #17367 @aeros https://bugs.python.org/issue27873
) Update docstring for `multiprocessing.Pool.map` to mention `pool.starmap()`. Prev PR: python#17367 @aeros https://bugs.python.org/issue27873 (cherry picked from commit eb48a45) Co-authored-by: An Long <aisk@users.noreply.github.com>
Update docstring for `multiprocessing.Pool.map` to mention `pool.starmap()`. Prev PR: #17367 @aeros https://bugs.python.org/issue27873 (cherry picked from commit eb48a45) Co-authored-by: An Long <aisk@users.noreply.github.com>
Update docstring for `multiprocessing.Pool.map` to mention `pool.starmap()`. Prev PR: #17367 @aeros https://bugs.python.org/issue27873 (cherry picked from commit eb48a45) Co-authored-by: An Long <aisk@users.noreply.github.com>
) Update docstring for `multiprocessing.Pool.map` to mention `pool.starmap()`. Prev PR: python#17367 @aeros https://bugs.python.org/issue27873
) Update docstring for `multiprocessing.Pool.map` to mention `pool.starmap()`. Prev PR: python#17367 @aeros https://bugs.python.org/issue27873
Co-authored-by: naught101 .
This PR is transfered from https://bugs.python.org/issue27873
https://bugs.python.org/issue27873