Author rhettinger
Recipients bar.harel, docs@python, giampaolo.rodola, josh.r, pitrou, rhettinger, serhiy.storchaka
Date 2020-06-23.04:27:34
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1592886454.9.0.514446014735.issue19270@roundup.psfhosted.org>
In-reply-to
Content
> Cancelling an unintended event instead of the one we wanted is a bug,
> and prevents me from using the library at all

That problem is certainly worth fixing even if we have to break a few eggs to do it (this module has been around for a very long time and almost certainly every little detail is being relied on by some users).

The docstring for enter() and enterabs() only promises that it returns an ID for an event and that the ID can be used to remove the event.

The regular docs for enter() and enterabs() make a stronger promise that they return an "event" but doesn't specify what that means.

The Event class name doesn't has a leading underscore but is not listed in __all__ and is not in the main docs.

The *queue* property is documented to return an ordered list of upcoming events which are defined to be named tuples with fields for: time, priority, action, arguments, kwargs.  That is the most specific and restrictive of the documented promises, one that presumably is being relied on in a myriad of ways (a repr suitable for logging, a pretty printable list, a sortable list, named access to fields, unpackable tuples, etc).

If we respect those published promises, one way to go is to add a unique identifier field to the Event class.  If the that unique id is consecutively numbered, it would also allow us to guarantee FIFO ordering (not sure whether that is actually need though).

Here's a preliminary sketch:

   scheduler.__init__():
+      self.event_sequence = 0   # updated as events are created
        ...

   scheduler.enterabs():
-       event = Event(time, priority, action, argument, kwargs)
+       self.event_sequence += 1
+       event = Event(time, priority, self.event_sequence, action, argument, kwargs)

- class Event(namedtuple('Event', 'time, priority, action, argument, kwargs')):
-    __slots__ = []
-    def __eq__(s, o): return (s.time, s.priority) == (o.time, o.priority)
-    def __lt__(s, o): return (s.time, s.priority) <  (o.time, o.priority)
-    def __le__(s, o): return (s.time, s.priority) <= (o.time, o.priority)
-    def __gt__(s, o): return (s.time, s.priority) >  (o.time, o.priority)
-    def __ge__(s, o): return (s.time, s.priority) >= (o.time, o.priority)
+ Event = namedtuple('Event', 'time, priority, sequence, action, argument, kwargs')

The user visible effects are:

* A new field would be displayed.
* Any code that constructs Event objects manually would break instantly.
* Any code using tuple unpacking on Event objects would break instantly.
* Slightly slower Event creation time.
* Faster heap updates (because native tuple ordering is faster than pure python rich comparisons).

However, outside the mention in the documentation for the *queue* property, Event objects are not part of the public API.  Also, we do have precedent for adding new fields without a negative consequence (argument and kwargs were added in Python 3.3).

An alternative solution for removing exact events (not for FIFO stability) is to have enter() and enterabs() return an opaque, unique event identifier.  Then the code for cancel() would need to be changed to something like:

    self._queue = [e for e in self._queue if id(e) != event]
    heapq.heapify(self._queue)

The user visible effects are:

* enter() and enterabs() no longer return an event object
  (it was implied that they would, but not explictly promised).
  Any code relying on an actual Event object being returned
  would break.
* cancel() would only accept the opaque IDs
  (again, it was only implied that actual Event objects would be used).
  Code would break that took Event objects out of the *queue* listing
  and passed those into cancel().
* cancel() would run a bit faster (because calling id() is cheaper
  that calling the pure python rich comparisons and having them
  extract the time and priority fields).

Either way breaks a few eggs but makes the module safer to use.
History
Date User Action Args
2020-06-23 04:27:34rhettingersetrecipients: + rhettinger, pitrou, giampaolo.rodola, docs@python, serhiy.storchaka, josh.r, bar.harel
2020-06-23 04:27:34rhettingersetmessageid: <1592886454.9.0.514446014735.issue19270@roundup.psfhosted.org>
2020-06-23 04:27:34rhettingerlinkissue19270 messages
2020-06-23 04:27:34rhettingercreate