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
return-ing within code timed with timeit.timeit causes wrong return value of timeit.timeit #62718
Comments
When I ran code below, it printed -1. The question is, the code in variable snippet, has wrong syntax, it can't be run. I think timeit should return only non-negative float type. from timeit import timeit
snippet="""
for i in range(10):
return -1
"""
print(timeit(snippet)) Thanks. |
Well, not sure if this is worth fixing, I think this is because timeit runs a modified version of the code using exec(), with the actual code to be timed within a function. (timeit runs something like this with exec(): def actual_code():
#The real code
for i in range(10): return -1
<some other code>
actual_code() |
I'm inclined to agree with Ramchandra. It might be worth a doc footnote, though. |
Agreed this should be closed as "won't fix". |
Rather than closing it, we converted it to a documentation issue. I think it is worth a footnote in the docs, since it is not obvious (without reading the source code) that a return statement will cause timeit to return an invalid result instead of raising a syntax error. |
Go ahead and add a footnote, but do consider that such footnotes are mostly just clutter. It doesn't help someone at the moment there is an invalid return value -- instead it just makes it so that afterward someone can say that it is documented. |
Added note to timeit function briefly explaining how to avoid it the issue and the cause |
New changeset 7e2708484ea5 by Andrew Kuchling in branch '3.4': |
The suggestion was to make this a footnote, not a note. Also, it should probably say that the stmt is executed inside a function, meaning that instead of being a syntax error it changes the return value of the internal timeit function. I understand Raymond's desire not to clutter the docs, but I consider the footnote worth it, not to pre-inform the user, but to let them know that it is not a bug if they check the docs *after* things don't work right. It may be naive of me to think that they would do so. |
I dislike footnotes and prefer to put things in the text whenever possible. The text for this change struck as relevant enough -- it notes a property of the *stmt* parameter -- that it doesn't belong in a footnote. (I'd be happy to just make it a paragraph instead of a highlighted note: I dislike notes too!) |
BTW, this change is also relevant to 2.7. Previously I wouldn't have bothered to commit it to 2.7 since the branch was largely closed, but now we're talking about some corporate maintainer eventually going through and backporting fixes to the newly-extended 2.7 branch. So should I go ahead and apply it to 2.7? |
OK, if you think it is worthwhile in the text, then sure. But yeah, not as a ..note. And yes, I think we should keep backporting relevant doc patches. Especially since Google results still land one on the 2.7 docs... |
May be add a guard against statements which can confuse timeit? These are not only "return", but "yield", and "break" and "continue" outside of a loop. Proposed patch checks that testing code can be compiled outside of a function. |
If there are no objections I'm going to commit the patch. |
New changeset e8db1cbe416b by Serhiy Storchaka in branch '2.7': New changeset a5769fa55791 by Serhiy Storchaka in branch '3.4': New changeset b0a686260b5d by Serhiy Storchaka in branch 'default': |
Buildbots are unhappy. Example: 1 test failed: Re-running test 'test_timeit' in verbose mode
test test_timeit crashed -- <type 'exceptions.ImportError'>: No module named support
Traceback (most recent call last):
File "./Lib/test/regrtest.py", line 901, in runtest_inner
File "/usr/home/buildbot/python/2.7.koobs-freebsd10/build/Lib/test/test_timeit.py", line 8, in <module>
from test.support import run_unittest
ImportError: No module named support
[44296 refs]
*** Error code 1 |
Already fixed in 617c226da195. Needs time for buildbots to rerun tests. I didn't noticed error when backported tests because there was local file support.py in my workspace. |
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:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: