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
timeit: Use thousands separators and print number of loops per second #63175
Comments
This patch includes:
The output is changed from this:
to that:
I'm still not sure about details of "28,870.783/s" part:
|
Oops, forgot to patch the tests, please find correct patch attached. |
I agree with both notes. Splitting the patch won't be a problem. As much as I don't fancy "," as thousands separator either - I just used what's in the standard library but I'll think about the best way of putting spaces there. |
Then you really want a non-breaking space ;-) (as a French person who's used to commas as decimal points, count me in the "commas as thousands separators are confusing" camp ;-))
Indeed, it probably doesn't make sense unless the number is < 100.
Yeah. |
I'm afraid the adding any separators will make some people angry. With a comma or space it no more a valid number in Python and many other languages and can't be copy/pasted and parsed. Actually I sometimes use small shell scripts to run "python -m timeit" and parsing results with grep, sed and awk. It is easer in simplest cases than writing it on Python (especially when different Python binaries used). I'm -0,1. |
I'm with Serhiy on this. So if separators are added, I would say they *must* be optional. Presumably if they are added they should be locale dependent :) All of which may well make it more complicated than it is worth. |
-1 from me, and I'm a comma-loving American ;-) I'm sure lots of code in the wild parses this output - Serhiy isn't the only one doing it. |
To me the point of this patch is adding number of loops per second information - using thousands separators was just an addition which I'm happy to drop. Please find attached 2 patches:
So:
|
Looks fine to me (though having a decimal space at the right and not at |
I do not find a space to be an acceptable separator, sorry. |
Antoine: I agree that it does look weird to have thousands separators at one place and not at the other but IMO it's still slightly better - the number formatted with separators is simply more readable that separator-less one. R. David Murray: what's the acceptable separator then? Modifying it to be locale-aware would make it inconsistent with the rest of the output (which, admittedly, is also the case with introducing space separator, dot still is the fraction separator though). I modified my patch to not have the separators at all, please find timeit-v3-actual-changes-no-separators.patch - if that's the condition for accepting this patch then so be it - the description of the patch would be "print number of loops per second" and the example output is: 10000 loops, best of 3: 34.6 usec per loop (28870 loops/s) |
My vote is no separators. But I'm just one vote :) |
Seconded. |
I'm fine with the change too. Tim, what do you think? Speaking of which, some examples were really done with an old machine: $ python -m timeit 'try:' ' str.__bool__' 'except AttributeError:' ' pass'
- 100000 loops, best of 3: 15.7 usec per loop
+ 1000000 loops, best of 3: 0.701 usec per loop (1425528 loops/s) :-) |
Inconsistency: "sec per loop" vs. "loops/s". |
That's right, I was actually wondering about it few minutes before you pointed it out. Find new patch attached. |
Reviewing this patch, it appears that the PEP-8 changes to timeit.py are already in the source and the discussion of the thousands separator is no longer an issue with the underscore changes in 3.6 (meaning underscore now seems the way to separate digits in large numbers, although it seems the vote was to remove all separators from timeit anyway). That would leave the |
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: