Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
Kain94 mannequin opened this issue Jan 21, 2011 · 23 comments
Closed

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

Kain94 mannequin opened this issue Jan 21, 2011 · 23 comments
Assignees
Labels
stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@Kain94
Copy link
Mannequin

Kain94 mannequin commented Jan 21, 2011

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 Kain94 mannequin added the performance Performance or resource usage label Jan 21, 2011
    @Kain94 Kain94 mannequin assigned collinwinter Jan 21, 2011
    @Kain94 Kain94 mannequin added the type-bug An unexpected behavior, bug, or error label Jan 21, 2011
    @Kain94
    Copy link
    Mannequin Author

    Kain94 mannequin commented Jan 21, 2011

    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

    @Kain94 Kain94 mannequin added stdlib Python modules in the Lib dir and removed performance Performance or resource usage labels Jan 21, 2011
    @abalkin
    Copy link
    Member

    abalkin commented Jan 21, 2011

    Works for me:

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

    (Tested with Python 3.1 and 3.2 on OSX.)

    @Kain94
    Copy link
    Mannequin Author

    Kain94 mannequin commented Jan 21, 2011

    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
    Copy link
    Member

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

    @amauryfa
    Copy link
    Member

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

    @amauryfa
    Copy link
    Member

    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
    Copy link
    Mannequin Author

    Kain94 mannequin commented Jan 21, 2011

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

    @pitrou
    Copy link
    Member

    pitrou commented Jan 21, 2011

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

    @pitrou pitrou changed the title Timer class inheritance issue Timer should be a class so that it can be derived Jan 21, 2011
    @pitrou pitrou added type-feature A feature request or enhancement and removed type-bug An unexpected behavior, bug, or error labels Jan 21, 2011
    @ericvsmith ericvsmith changed the title Timer should be a class so that it can be derived threading.Timer should be a class so that it can be derived Jan 21, 2011
    @merwok
    Copy link
    Member

    merwok commented Jan 21, 2011

    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
    Copy link
    Member

    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
    Copy link
    Member

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

    @merwok
    Copy link
    Member

    merwok commented Feb 16, 2011

    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
    Copy link
    Member

    merwok commented Jul 12, 2011

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

    @merwok
    Copy link
    Member

    merwok commented Jul 28, 2011

    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.

    @merwok merwok self-assigned this Jul 28, 2011
    @bitdancer
    Copy link
    Member

    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
    Copy link
    Member

    merwok commented Jul 28, 2011

    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
    Copy link
    Mannequin

    python-dev mannequin commented Jul 29, 2011

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

    @vstinner
    Copy link
    Member

    @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
    Copy link
    Member

    merwok commented Sep 15, 2011

    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
    Copy link
    Member

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

    Oh ok, cool, I missed the patches.

    @merwok
    Copy link
    Member

    merwok commented Jun 25, 2012

    Ping.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Oct 6, 2012

    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
    Copy link
    Member

    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.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    7 participants