Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(1)

#18983: Specify time unit for timeit CLI

Can't Edit
Can't Publish+Mail
Start Review
Created:
6 years, 4 months ago by storchaka+cpython
Modified:
5 years ago
Reviewers:
sky.kok, berker.peksag, robertc
CC:
tim.peters, Georg, AntoinePitrou, rbcollins, ezio.melotti, devnull_psf.upfronthosting.co.za, berkerpeksag, storchaka, Vincentdavis, vajrasky, jstasiak, Julian.Gindi, quentin.pradet_gmail.com
Visibility:
Public.

Patch Set 1 #

Patch Set 2 #

Total comments: 5

Patch Set 3 #

Total comments: 5

Patch Set 4 #

Patch Set 5 #

Total comments: 1

Patch Set 6 #

Patch Set 7 #

Patch Set 8 #

Total comments: 6
Unified diffs Side-by-side diffs Delta from patch set Stats Patch
Doc/library/timeit.rst View 1 2 3 4 5 6 7 2 chunks +5 lines, -1 line 2 comments Download
Lib/test/test_timeit.py View 1 2 3 4 5 6 7 1 chunk +20 lines, -0 lines 2 comments Download
Lib/timeit.py View 1 2 3 4 5 6 7 4 chunks +25 lines, -9 lines 2 comments Download

Messages

Total messages: 5
vajrasky
http://bugs.python.org/review/18983/diff/10124/Lib/test/test_timeit.py File Lib/test/test_timeit.py (right): http://bugs.python.org/review/18983/diff/10124/Lib/test/test_timeit.py#newcode291 Lib/test/test_timeit.py:291: def test_main_with_time_unit(self): You should test the ill input as ...
6 years, 3 months ago #1
storchaka_gmail.com
http://bugs.python.org/review/18983/diff/10260/Lib/timeit.py File Lib/timeit.py (right): http://bugs.python.org/review/18983/diff/10260/Lib/timeit.py#newcode256 Lib/timeit.py:256: time_unit = "default" You can use None as default ...
6 years, 3 months ago #2
storchaka_gmail.com
http://bugs.python.org/review/18983/diff/10268/Lib/timeit.py File Lib/timeit.py (right): http://bugs.python.org/review/18983/diff/10268/Lib/timeit.py#newcode268 Lib/timeit.py:268: sys.stderr.write("Unrecognized unit. Please select usec, msec, or sec.") This ...
6 years, 3 months ago #3
berkerpeksag
It would be good to have a release note in Doc/whatsnew/3.5.rst. http://bugs.python.org/review/18983/diff/10291/Doc/library/timeit.rst File Doc/library/timeit.rst (right): ...
5 years, 3 months ago #4
rbcollins
5 years ago #5
I've applied two of the review changes and disagree on the third (reason
inline). Running make test and will commit this shortly.

http://bugs.python.org/review/18983/diff/10291/Doc/library/timeit.rst
File Doc/library/timeit.rst (right):

http://bugs.python.org/review/18983/diff/10291/Doc/library/timeit.rst#newcode201
Doc/library/timeit.rst:201: 
On 2015/01/03 05:33:23, berkerpeksag wrote:
> Needs a versionadded directive.

Done.

http://bugs.python.org/review/18983/diff/10291/Lib/test/test_timeit.py
File Lib/test/test_timeit.py (right):

http://bugs.python.org/review/18983/diff/10291/Lib/test/test_timeit.py#newcod...
Lib/test/test_timeit.py:305: with captured_stderr() as error_stringio:
On 2015/01/03 05:33:23, berkerpeksag wrote:
> error_stringio -> stderr

This is actually consistent with the rest of the file, leaving as is for now.

http://bugs.python.org/review/18983/diff/10291/Lib/timeit.py
File Lib/timeit.py (right):

http://bugs.python.org/review/18983/diff/10291/Lib/timeit.py#newcode268
Lib/timeit.py:268: sys.stderr.write(
On 2015/01/03 05:33:23, berkerpeksag wrote:
> print("...", file=sys.stderr)
> 
> (Also, please remove "\n" in the message)

Done.
Sign in to reply to this message.

RSS Feeds Recent Issues | This issue
This is Rietveld 894c83f36cb7+