Roundup Robot added the comment:
New changeset 58776cc74e89 by Antoine Pitrou in branch 'default':
Issue #15837: add some tests for random.shuffle().
http://hg.python.org/cpython/rev/58776cc74e89
--
nosy: +python-dev
___
Python tracker
Antoine Pitrou added the comment:
I've committed the patch, thank you Alessandro.
--
assignee: rhettinger -
nosy: +pitrou
resolution: - fixed
stage: - committed/rejected
status: open - closed
___
Python tracker rep...@bugs.python.org
Serhiy Storchaka added the comment:
I am not sure that None as default should be documented. It's implementation
details (as third int argument) and can be silently changed in future
versions.
--
___
Python tracker rep...@bugs.python.org
Ezio Melotti added the comment:
I left a review on rietveld.
FWIW these are the results of the tests using timeit:
# with int=int
$ ./python -m timeit -s 'from random import random, shuffle; lst =
list(range(10))' 'shuffle(lst, random)'
10 loops, best of 3: 507 msec per loop
# without
Changes by Raymond Hettinger raymond.hettin...@gmail.com:
--
assignee: - rhettinger
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue15837
___
___
Raymond Hettinger added the comment:
The patch look fine as-is and it can be applied in 3.4. (BTW, I miss having a
Resolution status of Accepted, meaning that the patch passed review and is
ready to apply).
FWIW, I'll remove the int=int optimization in Py3.4. It doesn't provide much
Alessandro Moura added the comment:
Comparing the execution time with and without the int=int argument of this
command:
amoura@amoura-laptop:~/cpython$ time ./python -c from random import shuffle;
lst=list(range(100)); shuffle(lst); print (len(lst))
I get with int=int:
real0m13.755s
Serhiy Storchaka added the comment:
Third parameter (int) plays a role only in the presence of a second one
(random).
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue15837
___
R. David Murray added the comment:
No, it always has an effect. It means that the name 'int' is bound in locals
instead of being looked up via globals. That is what makes it a
micro-optimization (LOAD_FAST vs LOAD_GLOBAL, if you do a dis on the two
variants).
--
R. David Murray added the comment:
Oh, I see what you are saying. The lookup of int is only done if random is not
None. Yes, that is true.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue15837
R. David Murray added the comment:
If the optimization is actually useful, it can be preserved by just putting
'int=int' (with an 'optimization' comment :) before the loop.
--
___
Python tracker rep...@bugs.python.org
Alessandro Moura added the comment:
The int=int still makes no difference, but if the second argument is set to
random.random, we get a big speedup, regardless of whether the third argument
is there:
without int=int:
amoura@amoura-laptop:~/cpython$ time ./python -c import random;
Serhiy Storchaka added the comment:
This could be because of the many tests of whether the 2nd argument is None
in the loop.
This is because Random._randbelow (and therefore randrange, randint) is
relatively slow.
--
___
Python tracker
Alessandro Moura added the comment:
Yup. This is the result of simply eliminating the condition in the loop and
just using the second argument (for the purposes of testing this only):
amoura@amoura-laptop:~/cpython$ time ./python -c import random;
lst=list(range(100));
Serhiy Storchaka added the comment:
The int=int is probably some sort of micro-optimization and perhaps should be
removed.
Agree, this micro-optimization has no effect here.
--
nosy: +storchaka
___
Python tracker rep...@bugs.python.org
Alessandro Moura added the comment:
Sorry, here it is the patch.
--
keywords: +patch
Added file: http://bugs.python.org/file27082/random.patch
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue15837
New submission from Alessandro Moura:
Random.shuffle does not have a test in test_random.py; the attached patch adds
this test. In addition, I rewrote the documentation string for Random.shuffle,
which apparently did not reflect recent changes in the code and was
inconsistent with the
R. David Murray added the comment:
The patch seems to be missing.
The int=int is probably some sort of micro-optimization and perhaps should be
removed.
--
nosy: +r.david.murray, rhettinger
___
Python tracker rep...@bugs.python.org
18 matches
Mail list logo
