This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Raise on threading.Event.__bool__ due to ambiguous nature
Type: enhancement Stage: resolved
Components: Library (Lib) Versions: Python 3.11, Python 3.10
process
Status: closed Resolution: rejected
Dependencies: Superseder:
Assigned To: Nosy List: aa1371, rhettinger, sarf, steven.daprano
Priority: normal Keywords: patch

Created on 2021-04-24 10:39 by aa1371, last changed 2022-04-11 14:59 by admin. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 25575 closed sarf, 2021-04-24 10:43
Messages (5)
msg391771 - (view) Author: Art (aa1371) * Date: 2021-04-24 10:39
I'll sometimes find myself accidentally doing something like this (especially after a long break from using the threading module): 
```
stop_thread = threading.Event()
...
while not stop_thread:  # bug - bool(stop_thread) will always evaluate to True
    ...
```

Since the intention behind bool(event) is ambiguous and most likely often used improperly, I think that it would be a good idea to protect against this easy to produce bug, by overriding __bool__ to raise. There is precedent for this behavior in the popular numpy library, see here:
https://github.com/numpy/numpy/blob/623bc1fae1d47df24e7f1e29321d0c0ba2771ce0/numpy/core/src/multiarray/number.c#L829

Expanding on my thoughts:
1) Most operations on a threading.Event are associated with checking the truthiness of the underlying state of the Event. Meaning that there are many opportunities for bool(event) to be called improperly.
2) I can't think of any cases where  you would want to evaluate truthiness on anything other than the underlying "set" state of the Event. The one exception I can think of being the following (however, I believe this is generally accepted to be an anti-pattern, which I don't think should be considered a redeeming case for allowing bool(event)):
```
def my_func(event=None):
    event = event or threading.Event()
    ...
```
3) It is an easy addition to protect against this. Simply by raising in __bool__
4) The only backwards incompatibilities this could create are in cases where the event is being evaluated for truthiness incorrectly, and in the anti-pattern case described in point 2.
msg391774 - (view) Author: Steven D'Aprano (steven.daprano) * (Python committer) Date: 2021-04-24 12:20
This is not a bug fix, it is a change of behaviour ("enhancement"). All of 3.6 through 3.9 are in feature freeze. 3.10 is probably in feature freeze, but if not it is extremely close to it. So this can only go into 3.11 and (maybe) 3.10.

But I don't think this is a good enhancement. I am sorry that you keep making the mistake of wrongly assuming that you can use threading Events that way, but your mistake is not necessarily a flaw in the object.

Is there anything in the documentation that leads you to believe that Events are intended to be used that way?

The basic object model of Python is that all objects can be duck-typed as bools, that is they are either truthy or falsey. Values which represent "nothing" in some sense (empty containers, zero numbers, None) should evaluate as False; values which represent "something" (non-empty containers, non-zero numbers, arbitrary objects) should evaluate as True. This model goes back to the earliest days of Python 1.x before we even had a bool type or True and False singletons, and 1 and 0 where used instead.

`bool(obj)` is a fundamental operation, like `print(obj)`, which should never fail.

You say: "the intention behind bool(event) is ambiguous" but I don't believe that is correct. Unless documented differently, the intention behind bool(event) is:

- events are not containers;
- or numbers;
- but they are "things" (objects);
- and so they should evaluate as True.

So unless you can provide evidence for this ambiguity, or evidence that many other people make this same mistake, I will recommend this issue be closed.
msg391796 - (view) Author: Art (aa1371) * Date: 2021-04-24 20:12
Hi Steve, a couple things to preface my following comment. (1) Didn't mean to suggest that the current behavior is a bug. I don't think it is a bug, rather that it can easily lead to bugs. (2) Sorry for tagging the previous versions, I'm not familiar with the ticket system and didn't realize I was asking for (nor do I want) this to be changed in previous versions as well. I thought it just means what versions was this ticket relevant to.

I do realize and appreciate the basic object model regarding `bool(obj)`, and there is nothing in the threading documentation or the language as a whole that would lead me to believe that Events should be evaluated for their truthiness directly. However, I would like to expand on my case further before closing the ticket.

I believe there is a fundamental difference in the "perception" of what an Event object represents vs most other objects, *with respect to how bool should be evaluated on it*. It undeniably draws very clear parallels to a true boolean flag, and is often used as a surrogate for such. I realize it is more than that, that it's used for synchronization as well as other things, but the fact that Event is so often assigned to a variable called "flag" or a variable that implies a discrete boolean state (like my original stop_thread example), rather than a variable name that encompasses the full concept of an event, is a good indication that this is how people view/use the object. 

Given that the concept of Event and a boolean flag are so closely intertwined, I think that (but am *not* suggesting the following) it could even be considered appropriate for `bool(not_set_event)` to evaluate to False.  Again, I am not suggesting this, as I realize that an Event is more than just it's underlying "set" state. But, this is why I think that more often than not it is Ambiguous what a developer actually intended by directly evaluating such.

Now, in terms of what the current behavior enables us to do, in other words, by adopting this change, what abilities in the language/threading framework are we losing. The only thing I can think of is the ability to do this: `event = event or Event()`. I don't have statistics on this but I would make the assumption, and I believe it's a safe one, that the vast majority of situations where Event shows up in a boolean evaluated statement (e.g. if, while, not, and/or) is as `event.is_set()`. So much so, that I would even go so far as to make the assumption that there is a decently high probability that if someone does write `event or/and other_variable` it was done so in error. However, this is nothing but an assumption, with no evidence to back it up, so really the point I want to get across here is that there is not much utility in `bool(event)` and I don't think we're hindering the language in any way by forbidding it.

With respect to backwards compatibility, while it is not backwards compatible, it is very refactor friendly. First of all, there will not be many properly used cases (e.g. `event = event or Event()`) where this would show up in the first place. And second, since we're raising an exception rather than returning a different value, we won't introduce unexpected behavior to any existing use cases.

All this said, any bugs that this behavior can lead to, are most likely easily caught and resolved. But, I think that this change would be, and only be, a benefit to developing threaded applications in Python.
msg391818 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2021-04-24 23:03
Steven is right that this would be a behavior change.  It is also out of line with Python norms where all objects are born true and have to learn to be false with either __len__ or __bool__.  It is not a norm for bool(obj) to raise an exception.

Following Steven's suggestion, I'll mark this a closed.

Thank for the suggestion.
msg391835 - (view) Author: Art (aa1371) * Date: 2021-04-25 03:52
Understood. Thanks both, for taking the time to look.
History
Date User Action Args
2022-04-11 14:59:44adminsetgithub: 88095
2021-04-25 03:52:35aa1371setmessages: + msg391835
2021-04-24 23:03:42rhettingersetstatus: open -> closed

nosy: + rhettinger
messages: + msg391818

resolution: rejected
stage: patch review -> resolved
2021-04-24 20:12:04aa1371setmessages: + msg391796
2021-04-24 12:20:39steven.dapranosetversions: - Python 3.6, Python 3.7, Python 3.8, Python 3.9
nosy: + steven.daprano

messages: + msg391774

type: behavior -> enhancement
2021-04-24 10:43:07sarfsetkeywords: + patch
nosy: + sarf

pull_requests: + pull_request24294
stage: patch review
2021-04-24 10:39:43aa1371create