Raymond Hettinger <[email protected]> added the comment:
> 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.
----------
_______________________________________
Python tracker <[email protected]>
<https://bugs.python.org/issue19270>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe:
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
