-
-
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
gh-124285: document assumptions on __bool__/__len__ behaviour #124723
Conversation
| true. | ||
|
|
||
| Two successive calls to :meth:`!__bool__` on the same object must | ||
| return same value. |
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.
by another thread
... that calls some other object's method
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.
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.
|
As @AlexWaygood implied above, even builtin types can't provide the guarantee your current text for: if you call I think instead, we should say that in cases where the interpreter implicitly calls |
|
The language spec already says with regard to hashability:
So some interesting questions that come to mind for me here are:
If we think that well-behaved code should never do those things, and that doing those things could violate some assumptions made by Python in some places, then we could consider adding a similar note to the language spec for
But I'm not sure if it's worth doing so for Additionally, this sort-of feels like an implicit requirement for an awful lot of dunders, really. I would find it pretty surprising if |
Then this example will not satisfy to added remark: calls to
Do you have examples? While your points look reasonable, I'm not sure that such assumptions are actually used somewhere.
Well, in current version I have to add symmetric note for
There is a difference. The |
|
Maybe instead of saying something about |
|
I think the docs can just say that |
That's a more short version of the current statement, isn't? But I worry it might require explanation of the term. E.g. we don't expect that reader knows about complex numbers. |
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.
Suggested wording added inline. Mutating objects shared between threads is another way of hitting this particular piece of implementation defined behaviour, so I think it's best just to state it that way.
I think "no mutation (of this object, or any other object) " is the right expected invariant to specify, since that's the assumption that gets violated in the multi-threading case.
| Two successive calls to :meth:`!__bool__` on the same object must | ||
| return same value. |
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.
| Two successive calls to :meth:`!__bool__` on the same object must | |
| return same value. | |
| Note: to help optimize logical expressions, implementations are permitted to assume | |
| that calls to :meth:`!__bool__` will not mutate that object, nor any other object. | |
| While this expected invariant is not explicitly enforced, failing to abide | |
| by it will result in implementation dependent runtime behaviour. This | |
| implementation dependent behaviour may also be encountered when mutable | |
| objects are shared across threads without appropriate synchronization. |
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'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 comment
The reason will be displayed to describe this comment to others. Learn more.
is dependent on the user's choice of implementation.
Rather on bugs in the user code, if someone will implement __bool__(), doing crazy things (see issue). Added docs say that implementation may assume certain behaviour from the user code, just as for the __hash__() method (same hash value for equal objects - the invariant, which we also can't enforce, user code might break this).
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 comment
The 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 comment
The 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?
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 __bool__() (a shortened version of @ncoghlan suggestion):
| Two successive calls to :meth:`!__bool__` on the same object must | |
| return same value. | |
| Calls to :meth:!__bool__` shouldn't mutate any objects. |
| Two successive calls to :meth:`!__len__` on the same object must | ||
| return same value. |
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.
| Two successive calls to :meth:`!__len__` on the same object must | |
| return same value. | |
| Note: to help optimize logical expressions, implementations are permitted to assume | |
| that calls to :meth:`!__len__` will not mutate that object, nor any other object. | |
| While this expected invariant is not explicitly enforced, failing to abide | |
| by it will result in implementation dependent runtime behaviour. This | |
| implementation dependent behaviour may also be encountered when mutable | |
| objects are shared across threads without appropriate synchronization. |
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.
Here is a typo (__len__ -> __bool__). But if we are going with this lengthly wording - I think it's better just point to the __bool__ docs.
|
I think that I can't make progress on this. |
foo and 1 or 2: 3.12 newly convertsfooto bool twice #124285📚 Documentation preview 📚: https://cpython-previews--124723.org.readthedocs.build/