Skip to content

/ cpython

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

Fix typo in multiprocessing.pool.AsyncResult.successful doc. #17932

Merged
merged 1 commit into from Jan 15, 2020
Merged

Fix typo in multiprocessing.pool.AsyncResult.successful doc. #17932

merged 1 commit into from Jan 15, 2020

Conversation

@awecx
Copy link
Contributor

awecx commented Jan 9, 2020 •
edited by miss-islington

Since 3.7 successful raises a ValueError as explained in the next text block from the documentation:

Changed in version 3.7: If the result is not ready, ValueError is raised instead of AssertionError.

No issue associated with this PR.
Should be backported in 3.7 and 3.8.

Automerge-Triggered-By: @pitrou
@awecx
Fix typo in successful() reference doc.
Loading status checks…
293862e
@aeros
aeros approved these changes Jan 9, 2020
View changes
Copy link
Member

aeros left a comment •
edited

Thanks for the PR @awecx.

I confirmed from the source code that AsyncResult.successful() does indeed raise a ValueError instead of an AssertionError:

cpython/Lib/multiprocessing/pool.py

Lines 753 to 756 in a796d8e

def successful(self):
if not self.ready():
raise ValueError("{0!r} not ready".format(self))
return self._success

LGTM. This is also present in the 3.8 and 3.7 branches, so this PR should be backported to those versions as well.
@aeros aeros assigned pitrou and unassigned pitrou Jan 9, 2020
@aeros aeros requested a review from pitrou Jan 9, 2020
@aeros

This comment has been minimized.

Sign in to view
Copy link
Member

aeros commented Jan 9, 2020

Added @pitrou as a reviewer for the PR, since he's an active expert for the multiprocessing module.
@awecx

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

awecx commented Jan 10, 2020

Thanks for the PR @awecx.

I confirmed from the source code that AsyncResult.successful() does indeed raise a ValueError instead of an AssertionError:

cpython/Lib/multiprocessing/pool.py

Lines 753 to 756 in a796d8e

def successful(self):
if not self.ready():
raise ValueError("{0!r} not ready".format(self))
return self._success

LGTM. This is also present in the 3.8 and 3.7 branches, so this PR should be backported to those versions as well.

Thanks @aeros for the clarification. Indeed I should have proven the typo claim more clearly:
@pitrou if double check is necessary, you can run the following snippet:

from multiprocessing import Pool
from time import sleep

with Pool(processes=1) as pool:
    result = pool.apply_async(sleep, (10000,))
    result.successful()
@pitrou
pitrou approved these changes Jan 15, 2020
View changes
Copy link
Member

pitrou left a comment

+1
@miss-islington miss-islington merged commit dc0284e into python:master Jan 15, 2020
5 checks passed
5 checks passed
Docs
Details
Azure Pipelines PR #20200109.42 succeeded
Details
bedevere/issue-number Issue report skipped
bedevere/news "skip news" label found
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@miss-islington

This comment has been minimized.

Sign in to view
Copy link

miss-islington commented Jan 15, 2020

Thanks @awecx for the PR 🌮🎉.. I'm working now to backport this PR to: 3.7, 3.8.
🐍🍒⛏🤖
miss-islington added a commit to miss-islington/cpython that referenced this pull request Jan 15, 2020
@awecx @miss-islington
Fix typo in multiprocessing.pool.AsyncResult.successful doc. (pythonG…
54d6e06 
…H-17932)

Since 3.7 `successful` raises a `ValueError` as explained in the next text block from the documentation:

_Changed in version 3.7: If the result is not ready, ValueError is raised instead of AssertionError._

No issue associated with this PR.
Should be backported in 3.7 and 3.8.
(cherry picked from commit dc0284e)

Co-authored-by: Antoine <43954001+awecx@users.noreply.github.com>
@bedevere-bot

This comment has been minimized.

Sign in to view
Copy link

bedevere-bot commented Jan 15, 2020

GH-18015 is a backport of this pull request to the 3.8 branch.
miss-islington added a commit to miss-islington/cpython that referenced this pull request Jan 15, 2020
@awecx @miss-islington
Fix typo in multiprocessing.pool.AsyncResult.successful doc. (pythonG…
f7c86f0 
…H-17932)

Since 3.7 `successful` raises a `ValueError` as explained in the next text block from the documentation:

_Changed in version 3.7: If the result is not ready, ValueError is raised instead of AssertionError._

No issue associated with this PR.
Should be backported in 3.7 and 3.8.
(cherry picked from commit dc0284e)

Co-authored-by: Antoine <43954001+awecx@users.noreply.github.com>
@bedevere-bot

This comment has been minimized.

Sign in to view
Copy link

bedevere-bot commented Jan 15, 2020

GH-18016 is a backport of this pull request to the 3.7 branch.
miss-islington added a commit that referenced this pull request Jan 15, 2020
@miss-islington
[3.8] Fix typo in multiprocessing.pool.AsyncResult.successful doc. (G…
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
54abd28 
…H-17932) (GH-18015)

Since 3.7 `successful` raises a `ValueError` as explained in the next text block from the documentation:

_Changed in version 3.7: If the result is not ready, ValueError is raised instead of AssertionError._

No issue associated with this PR.
Should be backported in 3.7 and 3.8.
(cherry picked from commit dc0284e)


Co-authored-by: Antoine <43954001+awecx@users.noreply.github.com>

Automerge-Triggered-By: @pitrou
miss-islington added a commit that referenced this pull request Jan 15, 2020
@miss-islington
[3.7] Fix typo in multiprocessing.pool.AsyncResult.successful doc. (G…
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
74b95b6 
…H-17932) (GH-18016)

Since 3.7 `successful` raises a `ValueError` as explained in the next text block from the documentation:

_Changed in version 3.7: If the result is not ready, ValueError is raised instead of AssertionError._

No issue associated with this PR.
Should be backported in 3.7 and 3.8.
(cherry picked from commit dc0284e)


Co-authored-by: Antoine <43954001+awecx@users.noreply.github.com>

Automerge-Triggered-By: @pitrou
sthagen added a commit to sthagen/cpython that referenced this pull request Jan 15, 2020
@sthagen
Merge pull request #184 from python/master
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
a1b8199 
Fix typo in multiprocessing.pool.AsyncResult.successful doc. (pythonGH-17932)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pitrou pitrou

@aeros aeros

Assignees
No one assigned
Labels
🤖 automerge CLA signed skip issue skip news type-documentation
Projects
None yet
Milestone
No milestone
6 participants
@awecx @aeros @miss-islington @bedevere-bot @pitrou @the-knights-who-say-ni
You can’t perform that action at this time.