-
-
Notifications
You must be signed in to change notification settings - Fork 32.4k
Doc: Clarify the return type of Event.wait when timeout is used. #104168
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
Changes from 4 commits
81e3e82
1cd8571
a30382b
a34a9b7
f7f6623
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -996,9 +996,8 @@ method. The :meth:`~Event.wait` method blocks until the flag is true. | |
(or fractions thereof). | ||
|
||
This method returns ``True`` if and only if the internal flag has been set to | ||
true, either before the wait call or after the wait starts, so it will | ||
always return ``True`` except if a timeout is given and the operation | ||
times out. | ||
true, either before the wait call or after the wait starts. | ||
If a timeout is given and the operation times out, ``False`` will be returned. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't understand what is the expected return value if no timeout is given and set() was not called: the function just doesn't return and block until set() is called? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Several paragraphs above:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My rewrite combines paragraphs is a way that I think is clearer in less words. I think this ready to merge now. |
||
|
||
.. versionchanged:: 3.1 | ||
Previously, the method always returned ``None``. | ||
|
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.
"if the internal flag has been set" is vague. In practice, it means that the set() method was called, no?
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.
I agree that is is vague. I would also like to use that style of language, rather than talking about internal flags tbh.
I've re-written the docstring entirely based on the reviews, but haven't yet used the simpler language you suggested - the only hesitation is that we have to also mention
clear()
and the fact thatset()
may have been called beforewait()
if we want to be 100% precise (I'm not sure it helps with clarity to be so precise though).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.
The wait entry immediately follows the set and clear entries, so I think it should be clear enough that those are the functions for manipulating the 'internal flag'.