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

supplying an empty string to timeit causes an IndentationError #84847

Closed
simicode mannequin opened this issue May 18, 2020 · 10 comments
Closed

supplying an empty string to timeit causes an IndentationError #84847

simicode mannequin opened this issue May 18, 2020 · 10 comments
Labels
3.10 only security fixes docs Documentation in the Doc dir type-bug An unexpected behavior, bug, or error

Comments

@simicode
Copy link
Mannequin

simicode mannequin commented May 18, 2020

BPO 40670
Nosy @terryjreedy, @serhiy-storchaka, @tonybaloney, @DahlitzFlorian, @remilapeyre, @SimiCode
PRs
  • bpo-40670: Calling timeit.timeit with empty string uses 'pass' by default #20286
  • bpo-40670: Improve documentation for timeit function #20830
  • bpo-40670: More reliable validation of statements in timeit.Timer. #22358
  • 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 2021-05-18.16:42:39.542>
    created_at = <Date 2020-05-18.12:44:21.047>
    labels = ['type-bug', '3.10', 'docs']
    title = 'supplying an empty string to timeit causes an IndentationError'
    updated_at = <Date 2021-05-18.16:42:39.541>
    user = 'https://github.com/simicode'

    bugs.python.org fields:

    activity = <Date 2021-05-18.16:42:39.541>
    actor = 'iritkatriel'
    assignee = 'docs@python'
    closed = True
    closed_date = <Date 2021-05-18.16:42:39.542>
    closer = 'iritkatriel'
    components = ['Documentation']
    creation = <Date 2020-05-18.12:44:21.047>
    creator = 'edison.abahurire'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 40670
    keywords = ['patch']
    message_count = 10.0
    messages = ['369208', '369240', '369466', '369503', '369506', '369507', '369693', '369728', '369782', '377319']
    nosy_count = 8.0
    nosy_names = ['terry.reedy', 'docs@python', 'python-dev', 'serhiy.storchaka', 'anthonypjshaw', 'DahlitzFlorian', 'remi.lapeyre', 'edison.abahurire']
    pr_nums = ['20286', '20830', '22358']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue40670'
    versions = ['Python 3.10']

    @simicode
    Copy link
    Mannequin Author

    simicode mannequin commented May 18, 2020

    The Error can be evidenced below:

    >>>
    >>> import timeit
    >>>
    >>> timeit.timeit('', number=10000)
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "/usr/lib/python3.8/timeit.py", line 232, in timeit
        return Timer(stmt, setup, timer, globals).timeit(number)
      File "/usr/lib/python3.8/timeit.py", line 131, in __init__
        code = compile(src, dummy_src_name, "exec")
      File "<timeit-src>", line 7
        _t1 = _timer()
        ^
    IndentationError: expected an indented block
    >>>
    

    @remilapeyre
    Copy link
    Mannequin

    remilapeyre mannequin commented May 18, 2020

    Is this different than what you would expect?

    Supplying garbage to timeit will result in an error:

    >>> from timeit import timeit
    >>> timeit('weofinwofinwe')
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "/Library/Frameworks/Python.framework/Versions/3.8/lib/python3.8/timeit.py", line 232, in timeit
        return Timer(stmt, setup, timer, globals).timeit(number)
      File "/Library/Frameworks/Python.framework/Versions/3.8/lib/python3.8/timeit.py", line 176, in timeit
        timing = self.inner(it, self.timer)
      File "<timeit-src>", line 6, in inner
    NameError: name 'weofinwofinwe' is not defined

    If you want to time an empty loop, you can use:

    >>> timeit('pass', number=10000)
    0.0001043230000021822

    @simicode
    Copy link
    Mannequin Author

    simicode mannequin commented May 20, 2020

    Yes, This is unexpected

    In the first case:
    Supplying an empty string can also mean no code has been supplied. So it could be better treated as 'pass'.

    In the second case:
    Error messages are meant to inform you of what you have done wrong so that you fix it. I think raising an Indentation error is far from what a user who is calling that method would be expecting, it does not help them at-all.

    In the third case:
    Supplying an empty string to timeit directly in bash python -m timeit '' raises no such error. so we could say that fixing this will align the behavior in the two ways the function is run to be the same.

    @DahlitzFlorian
    Copy link
    Mannequin

    DahlitzFlorian mannequin commented May 21, 2020

    Calling timeit from command-line with the empty string defaults to 'pass'. I suggest to adopt this behaviour for calling timeit.timeit in the REPL as @edison.abahurire already suggested. I would be happy to submit a PR for it.

    @serhiy-storchaka
    Copy link
    Member

    Accepting an empty string in CLI is just an artifact of the implementation. There was no intention to support it. It will fail if pass a space:

    ./python -m timeit ' '
    

    or two empty arguments:
    IndentationError:
    ./python -m timeit '' ''

    I do not see this is an issue. Garbage in -- garbage out. IndentationError is a subclass of SyntaxError, so if you handle it programmatically, it does not matter.

    Of course we try to catch some user errors and provide informative traceback. timeit now correctly handles most of code which interferes with the control flow in functions and loops: 'return', 'yield', 'break', 'await'.

    But it is still possible to bypass the validation. For example:

    ./python -m timeit -s 'while False:' -s '    pass' '    break'
    

    I think there is an infinite number of ways to fool timeit. And an empty string is just one of them, not special enough to add a special handling in the code.

    @DahlitzFlorian
    Copy link
    Mannequin

    DahlitzFlorian mannequin commented May 21, 2020

    I see your point and agree with you. However, IMHO the CLI and the direct function call should behave the same way to not confuse users. The opened PR ensures that.

    @terryjreedy
    Copy link
    Member

    Let a wcs be a string consisting of only whitespace and comments.
    compile(wcs, '', 'exec') treats wcs the same as 'pass'.
    Hence, a file with only whitespace and comments is the same as 'pass'.
    compile(wcs, '', 'single'), for whatever reason, raises
    SyntaxError: unexpected EOF while parsing
    To get around this, a wcs input into IDLE's Shell, compiles with 'single', is replaced with 'pass' in codeop._maybe_compile, line 76. I presume the REPL does the same.

    If one thinks of parameter stmt as a top-level statement (or statements), it is reasonable to expect '' to be the same as 'pass'. If one knows that stmt will be embedded into a compound statement (whether 'while', 'for' or 'def' does not matter here) for repeated execution, then 'pass' is more obviously the minimal statement.

    It would have been better if the parameter name were 'suite', as suites can never be only whitespace. We cannot change this, but I suggest replacing

    The constructor takes a statement to be timed, an additional statement used for setup, and a timer function. Both statements default to 'pass'; the timer function is platform-dependent (see the module doc string). stmt and setup may also contain multiple statements separated by ; or newlines, as long as they don’t contain multi-line string literals.

    with the shorter, clearer, and updated

    The constructor takes suite of statement to be timed, an additional suite used for setup, and a timer function (default time.perf_counter). Both suites default to 'pass' and may not contain multi-line string literals.

    Since 3.3, the default timer is platform-independent, at least from a user viewpoint, and not mentioned in timeit.__doc__. Suites can always have multiple statments separated by ; and \n. The only needed qualification is the default and the restriction.

    @terryjreedy terryjreedy added 3.10 only security fixes labels May 23, 2020
    @DahlitzFlorian
    Copy link
    Mannequin

    DahlitzFlorian mannequin commented May 23, 2020

    @terry.reedy sorry if I misunderstood you, but it seems to me that you agree with the proposed changes?

    @terryjreedy
    Copy link
    Member

    I intended to say that the current behavior would be less puzzling if the doc were changed as I suggest. I think that a doc change would be sufficient.

    @iritkatriel iritkatriel added the docs Documentation in the Doc dir label Sep 22, 2020
    @iritkatriel iritkatriel added type-bug An unexpected behavior, bug, or error docs Documentation in the Doc dir labels Sep 22, 2020
    @iritkatriel iritkatriel added the type-bug An unexpected behavior, bug, or error label Sep 22, 2020
    @serhiy-storchaka
    Copy link
    Member

    New changeset 557b9a5 by Serhiy Storchaka in branch 'master':
    bpo-40670: More reliable validation of statements in timeit.Timer. (GH-22358)
    557b9a5

    @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
    3.10 only security fixes docs Documentation in the Doc dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants