threading.Timer should be a class so that it can be derived

[Kain94]

BPO	10968
Nosy	@gvanrossum, @amauryfa, @abalkin, @pitrou, @vstinner, @ericvsmith, @giampaolo, @merwok, @bitdancer
Files	issue10968.png threading-classes.diff threading-3.3-doc.diff

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/merwok'
closed_at = <Date 2012-10-06.18:43:13.812>
created_at = <Date 2011-01-21.01:44:59.409>
labels = ['type-feature', 'library']
title = 'threading.Timer should be a class so that it can be derived'
updated_at = <Date 2012-10-06.18:43:13.811>
user = 'https://bugs.python.org/Kain94'

bugs.python.org fields:

activity = <Date 2012-10-06.18:43:13.811>
actor = 'r.david.murray'
assignee = 'eric.araujo'
closed = True
closed_date = <Date 2012-10-06.18:43:13.812>
closer = 'r.david.murray'
components = ['Library (Lib)']
creation = <Date 2011-01-21.01:44:59.409>
creator = 'Kain94'
dependencies = []
files = ['20479', '22631', '22784']
hgrepos = []
issue_num = 10968
keywords = ['patch', 'needs review']
message_count = 23.0
messages = ['126677', '126684', '126719', '126722', '126723', '126728', '126730', '126732', '126763', '126771', '126808', '128645', '140190', '141294', '141299', '141300', '141349', '143997', '144079', '144082', '163900', '172236', '172237']
nosy_count = 13.0
nosy_names = ['gvanrossum', 'collinwinter', 'amaury.forgeotdarc', 'belopolsky', 'pitrou', 'vstinner', 'eric.smith', 'giampaolo.rodola', 'eric.araujo', 'r.david.murray', 'Kain94', 'Martijn.van.Oosterhout', 'python-dev']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue10968'
versions = ['Python 3.3']

[Kain94]

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

[abalkin]

Works for me:

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

(Tested with Python 3.1 and 3.2 on OSX.)

[Kain94]

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"

[amauryfa]

I can't reproduce the error. Do you have a script that shows the issue?
What is the complete traceback?

[amauryfa]

Ah, got it. It's about threading.Timer, which looks like a class, but is actually a function.

[amauryfa]

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().

[Kain94]

Yes that's it. I Should precise it. I've taken a screenshot from my python's interpreter to spot it.

[pitrou]

AFAIK this is by design. Not that I agree with this decision anyway.

[merwok]

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.

[gvanrossum]

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.

[bitdancer]

See also bpo-5831. That should probably be closed as a dup of this since this has an explanation.

[merwok]

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?

[merwok]

Attached patch removes the indirection functions; the _Verbose shenanigans are left alone. The test suite passes; I haven’t edited the doc yet.

[merwok]

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.

[bitdancer]

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.

[merwok]

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.

[python-dev]

New changeset 7f606223578a by Éric Araujo in branch 'default':
Remove indirection in threading (issue bpo-10968).
http://hg.python.org/cpython/rev/7f606223578a

[vstinner]

@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 bpo-12960.

[merwok]

Victor: Yes, I know the doc needs an update, that’s why I posted a patch six weeks ago and asked for a review.

[vstinner]

that’s why I posted a patch six weeks ago and asked for a review

Oh ok, cool, I missed the patches.

[merwok]

Ping.

[python-dev]

New changeset 98499371ca53 by R David Murray in branch '3.3':
bpo-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 bpo-10968: commit threading doc changes and corresponding whatsnew entry.
http://hg.python.org/cpython/rev/ad435964fc9c

[bitdancer]

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.