-
-
Notifications
You must be signed in to change notification settings - Fork 33.2k
gh-124285: document assumptions on __bool__/__len__ behaviour #124723
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 all commits
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 | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -2037,12 +2037,15 @@ Basic customization | |||||||||||||||||||||||
| .. index:: single: __len__() (mapping object method) | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| Called to implement truth value testing and the built-in operation | ||||||||||||||||||||||||
| ``bool()``; should return ``False`` or ``True``. When this method is not | ||||||||||||||||||||||||
| :func:`bool`; should return ``False`` or ``True``. When this method is not | ||||||||||||||||||||||||
| defined, :meth:`~object.__len__` is called, if it is defined, and the object is | ||||||||||||||||||||||||
| considered true if its result is nonzero. If a class defines neither | ||||||||||||||||||||||||
| :meth:`!__len__` nor :meth:`!__bool__`, all its instances are considered | ||||||||||||||||||||||||
| true. | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| Two successive calls to :meth:`!__bool__` on the same object must | ||||||||||||||||||||||||
| return same value. | ||||||||||||||||||||||||
|
Comment on lines
+2046
to
+2047
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.
Suggested change
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'm unsure if this needs to be explicitly documented as the behavior is dependent on the user's choice of implementation. 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.
Rather on bugs in the user code, if someone will implement PS: I think that part of discussion rather belongs to the issue thread, which has some other arguments on why we want document this. See e.g. this. 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. Should the note warn against side effects in general (like I/O), not just mutation? 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.
Side effects, including in fact object mutation - are fine, unless they break idempotence. But I think we don't loose something practically relevant if just forbid mutation of any objects in
Suggested change
|
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| .. _attribute-access: | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
|
|
@@ -2947,6 +2950,9 @@ through the object's keys; for sequences, it should iterate through the values. | |||||||||||||||||||||||
| :meth:`~object.__bool__` method and whose :meth:`!__len__` method returns zero is | ||||||||||||||||||||||||
| considered to be false in a Boolean context. | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| Two successive calls to :meth:`!__len__` on the same object must | ||||||||||||||||||||||||
| return same value. | ||||||||||||||||||||||||
|
Comment on lines
+2953
to
+2954
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.
Suggested change
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. Here is a typo ( |
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| .. impl-detail:: | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| In CPython, the length is required to be at most :data:`sys.maxsize`. | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
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.
Not if the object is mutated.
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.
But that means - some other method was called on the object in between. Thus, calls to
__bool__weren't actually successive.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 object could have been mutated by another thread between the two calls...
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.
... that calls some other object's method
:)
Uh oh!
There was an error while loading. Please reload this page.
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.
Hmm, how about this: "The
__bool__method can't mutate any objects."? (Ditto for__len__.)This probably is a more strong requirement than actually need in current optimizations, but I doubt it blocks something useful.