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

Pass a namespace to timeit #46779

Closed
peterotten mannequin opened this issue Apr 1, 2008 · 20 comments
Closed

Pass a namespace to timeit #46779

peterotten mannequin opened this issue Apr 1, 2008 · 20 comments
Labels
stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@peterotten
Copy link
Mannequin

peterotten mannequin commented Apr 1, 2008

BPO 2527
Nosy @birkenfeld, @rhettinger, @abalkin, @pitrou, @stevendaprano
Files
  • diff_against_2_5_1.txt: diff against 2.5.1
  • timeit_global_arg.patch
  • timeit_global_arg_v2.patch
  • 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 = None
    closed_at = <Date 2014-08-23.03:25:00.464>
    created_at = <Date 2008-04-01.12:13:46.612>
    labels = ['type-feature', 'library']
    title = 'Pass a namespace to timeit'
    updated_at = <Date 2014-08-23.15:28:53.867>
    user = 'https://bugs.python.org/peterotten'

    bugs.python.org fields:

    activity = <Date 2014-08-23.15:28:53.867>
    actor = 'roippi'
    assignee = 'none'
    closed = True
    closed_date = <Date 2014-08-23.03:25:00.464>
    closer = 'pitrou'
    components = ['Library (Lib)']
    creation = <Date 2008-04-01.12:13:46.612>
    creator = 'peter.otten'
    dependencies = []
    files = ['9916', '36409', '36432']
    hgrepos = []
    issue_num = 2527
    keywords = ['patch']
    message_count = 20.0
    messages = ['64805', '64812', '64814', '64816', '64838', '64857', '80477', '81215', '81267', '85689', '224933', '225508', '225509', '225621', '225627', '225633', '225639', '225724', '225725', '225750']
    nosy_count = 10.0
    nosy_names = ['georg.brandl', 'rhettinger', 'peter.otten', 'belopolsky', 'pitrou', 'LambertDW', 'steven.daprano', 'stefanv', 'python-dev', 'roippi']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue2527'
    versions = ['Python 3.5']

    @peterotten
    Copy link
    Mannequin Author

    peterotten mannequin commented Apr 1, 2008

    I'd like to suggest a different approach than the one taken in rev.
    54348 to improve timeit's scripting interface: allow passing it a
    namespace. Reasons:

    - It has smaller overhead for functions that take an argument:
    >>> def f(a): pass
    ...
    # trunk
    >>> min(ht.Timer(lambda f=f: f(42)).repeat())
    0.54068493843078613
    # my patch
    >>> min(mt.Timer("f(42)", ns=dict(f=f)).repeat())
    0.29009604454040527
    • it is more flexible. Example:
      # working code, requires matplotlib
      from timeit import Timer
      from time import sleep
    def linear(i):
        sleep(.05*i)
    def quadratic(i):
        sleep(.01*i**2)
    
    x = range(10)
    y = []
    for a in x:
        y.append([min(Timer("f(a)", ns=dict(f=f, a=a)).repeat(1, 1))
                  for f in linear, quadratic])
    
    from pylab import plot, show
    plot(x, y)
    show()

    The above code works unaltered inside a function, unlike the hacks
    using "from __main__ import ...".

    • the implementation is simpler and should be easy to maintain.

    The provided patch is against 2.5.1. If it has a chance of being
    accepted I'm willing to jump through the necessary hoops:
    documentation, tests, etc.

    @peterotten peterotten mannequin added stdlib Python modules in the Lib dir type-feature A feature request or enhancement labels Apr 1, 2008
    @abalkin
    Copy link
    Member

    abalkin commented Apr 1, 2008

    A more general approach would be to add both 'locals' and 'globals' to
    be used by exec. At least, I would change 'ns' to 'locals'.

    @abalkin
    Copy link
    Member

    abalkin commented Apr 1, 2008

    On the second thought, I actually wanted Timer to mimic eval without
    realizing that eval uses positional rather than keywords arguments.
    'locals' is obviously a bad choice for the keyword parameter because it
    masks locals() builtin. Maybe 'local_namespace'?

    @pitrou
    Copy link
    Member

    pitrou commented Apr 1, 2008

    Generally, when I use timeit from the interpreter prompt, I use "from
    __main__ import *" as the setup code string. Then I can use all
    currently defined global symbols directly :)

    @peterotten
    Copy link
    Mannequin Author

    peterotten mannequin commented Apr 2, 2008

    Alexander, I'm fine with a more specific argument name. ns was what
    the Timer already used internally.

    Antoine, from __main__ import name1, ..., nameN works fine on the
    command line, but inside a function you'd have to declare the names
    you want to pass to the Timer as globals which I find a bit clumsy.
    Apart from giving a syntax warning a star-import affects the generated
    bytecode and produces the (slower) LOAD_NAME instead of LOAD_FAST.

    @abalkin
    Copy link
    Member

    abalkin commented Apr 2, 2008

    On Wed, Apr 2, 2008 at 2:42 AM, Peter Otten <report@bugs.python.org> wrote:

    Alexander, I'm fine with a more specific argument name. ns was what
    the Timer already used internally.

    Maybe it should be "locals" after all. It does not look like the
    conflict with builtin locals() is an issue. Note that this is what
    __import__ uses. I still recommend adding globals argument as well
    for completeness and more accurate timings when timed code uses
    globals.

    @lambertdw
    Copy link
    Mannequin

    lambertdw mannequin commented Jan 24, 2009

    This note is simply a reminder that Antoine's 'from __main__ import *'
    solution fails in python3. Also, resolution of this issue probably
    could incorporate bpo-1397474.

    >>> import timeit
    >>> timeit.timeit('None','from __main__ import *')
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "/usr/local/lib/python3.0/timeit.py", line 227, in timeit
        return Timer(stmt, setup, timer).timeit(number)
      File "/usr/local/lib/python3.0/timeit.py", line 135, in __init__
        code = compile(src, dummy_src_name, "exec")
      File "<timeit-src>", line 2
    SyntaxError: import * only allowed at module level

    @rhettinger rhettinger self-assigned this Jan 27, 2009
    @birkenfeld birkenfeld assigned pitrou and unassigned rhettinger Feb 5, 2009
    @rhettinger
    Copy link
    Contributor

    rhettinger commented Feb 5, 2009

    Georg, why did you reassign this?

    @birkenfeld
    Copy link
    Member

    birkenfeld commented Feb 6, 2009

    I'm sorry, this should have been another issue. Reassigning to you.

    @birkenfeld birkenfeld assigned rhettinger and unassigned pitrou Feb 6, 2009
    @rhettinger
    Copy link
    Contributor

    rhettinger commented Apr 7, 2009

    See related discussion in bpo-5441 and bpo-1397474.

    @rhettinger rhettinger removed their assignment Apr 7, 2009
    @pitrou
    Copy link
    Member

    pitrou commented Aug 6, 2014

    Would still be nice to have something like this. The timeit module API is still crippled, especially now that "from __main__ import *" doesn't work in a function anymore.

    @roippi
    Copy link
    Mannequin

    roippi mannequin commented Aug 18, 2014

    Attached is a patch that adds a 'global' kwarg to the Timeit constructor, which does pretty much what it says on the tin: specifies a global namespace that exec() will use.

    I originally had a 'locals' arg as well (to mirror the signature of eval/exec), but realized that the local vars I was passing to exec were not available to the inner function. Reason: the timeit module compiles/execs a *closure*, and closed-over variables and exec() simply do not play nicely. Possible workarounds were to munge locals() into the globals() dict, or to somehow inject the variables in the locals dict into the closure. I found neither of these options superior to simply not including a locals argument, for reasons of Least Surprise and maintainability.

    Patch includes some basic tests and documentation. I am particularly uncomfortable with writing docs so those very likely need some polish.

    @roippi
    Copy link
    Mannequin

    roippi mannequin commented Aug 18, 2014

    Correction, the name of the argument is 'globals', not 'global'.

    @pitrou
    Copy link
    Member

    pitrou commented Aug 21, 2014

    Ben, thanks for the patch. Have you signed a contributor's agreement? You can find it at https://www.python.org/psf/contrib/contrib-form/

    @roippi
    Copy link
    Mannequin

    roippi mannequin commented Aug 22, 2014

    I did sign one right after I submitted the patch. Takes a few days for the asterisks to propagate I guess :)

    @pitrou
    Copy link
    Member

    pitrou commented Aug 22, 2014

    Ah, good. The patch looks fine to me, except that you should add "versionchanged" tags in the documentation for the added parameter.

    @roippi
    Copy link
    Mannequin

    roippi mannequin commented Aug 22, 2014

    Ah yes.

    New patch improves the docs.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Aug 23, 2014

    New changeset e0f681f4ade3 by Antoine Pitrou in branch 'default':
    Issue bpo-2527: Add a *globals* argument to timeit functions, in order to override the globals namespace in which the timed code is executed.
    http://hg.python.org/cpython/rev/e0f681f4ade3

    @pitrou
    Copy link
    Member

    pitrou commented Aug 23, 2014

    Thank you, Ben! Your patch is now pushed to the default branch.

    @pitrou pitrou closed this as completed Aug 23, 2014
    @roippi
    Copy link
    Mannequin

    roippi mannequin commented Aug 23, 2014

    Thanks Antoine. Cheers :-)

    @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

    4 participants