classification
Title: threading.Timer should be a class so that it can be derived
Type: enhancement Stage: resolved
Components: Library (Lib) Versions: Python 3.3
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: eric.araujo Nosy List: Kain94, Martijn.van.Oosterhout, amaury.forgeotdarc, belopolsky, collinwinter, eric.araujo, eric.smith, giampaolo.rodola, gvanrossum, haypo, pitrou, python-dev, r.david.murray
Priority: normal Keywords: needs review, patch

Created on 2011-01-21 01:44 by Kain94, last changed 2012-10-06 18:43 by r.david.murray. This issue is now closed.

Files
File name Uploaded Description Edit
issue10968.png Kain94, 2011-01-21 12:13
threading-classes.diff eric.araujo, 2011-07-12 13:45 review
threading-3.3-doc.diff eric.araujo, 2011-07-28 13:57 review
Messages (23)
msg126677 - (view) Author: Benjamin VENELLE (Kain94) Date: 2011-01-21 01:48
Hi,

Due to Timer's object creation behavior, it is impossible to subclass it. This kind of declaration will result to an exception raising:

class A(Timer): pass
msg126684 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2011-01-21 02:28
Works for me:

>>> from timeit import Timer
>>> class A(Timer): pass
... 

(Tested with Python 3.1 and 3.2 on OSX.)
msg126719 - (view) Author: Benjamin VENELLE (Kain94) Date: 2011-01-21 11:38
I've tested it with Python 3.1.2 under Windows 7 32 bits. It raises the following TypeError exception "function() argument 1 must be code, not str"
msg126722 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2011-01-21 11:54
I can't reproduce the error. Do you have a script that shows the issue?
What is the complete traceback?
msg126723 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2011-01-21 11:57
Ah, got it. It's about threading.Timer, which looks like a class, but is actually a function.
msg126728 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2011-01-21 12:10
It seems to be by design: from the documentation:
http://docs.python.org/py3k/library/threading.html
"Timer is a subclass of Thread", and a Thread subclass should "only override the __init__() and run() methods".

What are you trying to do here? overriding run() is probably wrong; and overriding __init__ is better done by passing the correct parameters to threading.Timer().
msg126730 - (view) Author: Benjamin VENELLE (Kain94) Date: 2011-01-21 12:13
Yes that's it. I Should precise it. I've taken a screenshot from my python's interpreter to spot it.
msg126732 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-01-21 12:21
AFAIK this is by design. Not that I agree with this decision anyway.
msg126763 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-01-21 18:36
Adding Guido, who checked the module in, to nosy.

Guido: Could you tell us whether the fake classes in threading.py should stay as is or can be fixed?  The whole _Verbose business and Thing/_Thing indirection seem a bit outdated and unneeded.
msg126771 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2011-01-21 18:59
IIRC:

The design started out this way because it predates new-style classes. When this was put in one couldn't subclass extension types, and there were plans/hopes to replace some of the lock types with platform-specific built-in versions on some platforms.

Since it is now possible to write subclassable extension types the Thing/_Thing design is no longer needed.

I'm not sure about the _Verbose hacks, if it is deemed not useful I'm fine with letting it go.
msg126808 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2011-01-22 00:56
See also issue 5831.  That should probably be closed as a dup of this since this has an explanation.
msg128645 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-02-16 10:45
One concern expressed on a duplicate report by Martijn van Oosterhout:

> Note this is a behaviour change. Under the old scheme (Foo is a class)
>
> Foo.timerclass = Timer
>
> created a method, whereas now it will just assign the class as an
> attribute. To work around this you had to use _Timer. Will that dummy
> class remain as an alias to avoid breaking code (in 2.7 at least)?

Stable versions (2.7, 3.1, soon 3.2) won’t get this change.  They may get a doc patch to warn people about the fake class/factory thing.


In the py3k documentation for threading, some of the fake classes are documented as factories (Event) and other ones as classes (Timer); do you people think there would be harm in cleaning up all those outdated shenanigans for 3.3, with due versionchanged notes in the doc?
msg140190 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-07-12 13:45
Attached patch removes the indirection functions; the _Verbose shenanigans are left alone.  The test suite passes; I haven’t edited the doc yet.
msg141294 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-07-28 13:57
I’ve committed the cleanup to my 3.3 clone and will push this week.

Here’s a doc patch.  Before my patch, the various classes were documented in two parts: one entry with the factory function (e.g. Thread), without index reference, and one section (e.g. “Thread Objects”), which used a class directive (and thus index target) for most but not all classes.

After my patch, all classes are documented with a class directive, in their section (i.e. “Thread Objects”), with a versionchanged note informing that the name used to be that of a factory function.  The only remaining glitch is that the “X Objects” sections start with a description of the class’ use, which references methods with constructs like :meth:`run`, which cannot be turned into links as Sphinx lacks context: the class directive only happens after.  I could move the class directives right after the heading (“X Objects”), so that the meth roles get turned into links.
msg141299 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2011-07-28 14:20
You ought to be able to use either a context directive (I forget its name and syntax), or the full reference syntax (:meth:`~threading.Thread.run`) to make those links work without moving things around.
msg141300 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-07-28 14:24
> You ought to be able to use either a context directive (I forget its
> name and syntax),

Hm, do you mean something similar to currentmodule?

> or the full reference syntax (:meth:`~threading.Thread.run`) to make
> those links work without moving things around.

Yes, Thread.run would work, but I find that inelegant.
msg141349 - (view) Author: Roundup Robot (python-dev) Date: 2011-07-29 12:35
New changeset 7f606223578a by Éric Araujo in branch 'default':
Remove indirection in threading (issue #10968).
http://hg.python.org/cpython/rev/7f606223578a
msg143997 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2011-09-13 23:43
@eric: The doc has to be updated:

http://docs.python.org/dev/library/threading.html#threading.activeCount

"threading.Condition()
A factory function that returns a new condition variable object. A condition variable allows one or more threads to wait until they are notified by another thread.

See Condition Objects."

--

See also (maybe) issue #12960.
msg144079 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-09-15 14:49
Victor: Yes, I know the doc needs an update, that’s why I posted a patch six weeks ago and asked for a review.
msg144082 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2011-09-15 14:56
> that’s why I posted a patch six weeks ago and asked for a review

Oh ok, cool, I missed the patches.
msg163900 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-06-25 06:07
Ping.
msg172236 - (view) Author: Roundup Robot (python-dev) Date: 2012-10-06 18:38
New changeset 98499371ca53 by R David Murray in branch '3.3':
#10968: commit threading doc changes and corresponding whatsnew entry.
http://hg.python.org/cpython/rev/98499371ca53

New changeset ad435964fc9c by R David Murray in branch 'default':
merge #10968: commit threading doc changes and corresponding whatsnew entry.
http://hg.python.org/cpython/rev/ad435964fc9c
msg172237 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2012-10-06 18:43
I have now committed (a revised version of) the doc changes.

Like I said in the commit message, it is unfortunate that the underscore names were not kept as aliases and that RLock wasn't also converted to a class, but it is too late to fix that in 3.3.  If someone wants to do RLock in 3.4 they should open a new issue.
History
Date User Action Args
2012-10-06 18:43:13r.david.murraysetstatus: open -> closed
resolution: fixed
messages: + msg172237

stage: patch review -> resolved
2012-10-06 18:38:29python-devsetmessages: + msg172236
2012-06-25 06:07:09eric.araujosetkeywords: + needs review

messages: + msg163900
stage: patch review
2011-09-15 14:56:02hayposetmessages: + msg144082
2011-09-15 14:49:51eric.araujosetmessages: + msg144079
2011-09-13 23:43:05hayposetnosy: + haypo
messages: + msg143997
2011-07-29 12:35:17python-devsetnosy: + python-dev
messages: + msg141349
2011-07-28 14:24:42eric.araujosetmessages: + msg141300
2011-07-28 14:20:16r.david.murraysetmessages: + msg141299
2011-07-28 13:57:17eric.araujosetfiles: + threading-3.3-doc.diff
assignee: eric.araujo
messages: + msg141294
2011-07-12 13:45:57eric.araujosetfiles: + threading-classes.diff
keywords: + patch
messages: + msg140190
2011-02-16 10:45:50eric.araujosetnosy: + Martijn.van.Oosterhout
messages: + msg128645
2011-01-22 20:23:21giampaolo.rodolasetnosy: + giampaolo.rodola
2011-01-22 18:32:29eric.araujolinkissue5831 superseder
2011-01-22 00:56:58r.david.murraysetnosy: + r.david.murray
messages: + msg126808
2011-01-21 18:59:44gvanrossumsetnosy: gvanrossum, collinwinter, amaury.forgeotdarc, belopolsky, pitrou, eric.smith, eric.araujo, Kain94
messages: + msg126771
2011-01-21 18:36:12eric.araujosetnosy: + eric.araujo, gvanrossum
messages: + msg126763
2011-01-21 13:57:21eric.smithsetnosy: + eric.smith

title: Timer should be a class so that it can be derived -> threading.Timer should be a class so that it can be derived
2011-01-21 12:21:10pitrousetassignee: collinwinter -> (no value)
type: behavior -> enhancement

title: Timer class inheritance issue -> Timer should be a class so that it can be derived
nosy: + pitrou
versions: + Python 3.3, - Python 3.1
messages: + msg126732
2011-01-21 12:13:15Kain94setfiles: + issue10968.png
nosy: collinwinter, amaury.forgeotdarc, belopolsky, Kain94
messages: + msg126730
2011-01-21 12:10:35amaury.forgeotdarcsetnosy: collinwinter, amaury.forgeotdarc, belopolsky, Kain94
messages: + msg126728
2011-01-21 11:57:29amaury.forgeotdarcsetnosy: collinwinter, amaury.forgeotdarc, belopolsky, Kain94
messages: + msg126723
2011-01-21 11:54:24amaury.forgeotdarcsetnosy: + amaury.forgeotdarc
messages: + msg126722
2011-01-21 11:38:39Kain94setnosy: collinwinter, belopolsky, Kain94
messages: + msg126719
2011-01-21 02:28:50belopolskysetnosy: + belopolsky
messages: + msg126684
2011-01-21 01:48:19Kain94setnosy: collinwinter, Kain94
messages: + msg126677
components: + Library (Lib), - Benchmarks
versions: + Python 3.1
2011-01-21 01:44:59Kain94create