homepage

In randrange there is the note, "Do not supply the 'int' argument." That could probably be added to shuffle, too.

----
> nosy: +rhettinger
> title: add not about int to shuffle -> add "Do not supply int argument" to random.shuffle docstring
> versions: +Python 2.7, Python 3.3
>
Thanks. I couldn't even figure out what my own subject meant! (I see
that the "e" was missing from "note"; yours is much better.
Chris

I added the extra information to the docstring for the shuffle method and attached a patch.

On Tue, Jun 12, 2012 at 1:34 AM, Michael Driscoll
<report@bugs.python.org> wrote:
>
> Michael Driscoll <mike@pythonlibrary.org> added the comment:
>
> I added the extra information to the docstring for the shuffle method and attached a patch.
Thanks Michael (and Python team)!
Chris

New changeset 82bdd5fc7a71 by Senthil Kumaran in branch '2.7':
Improve the docstring of random.shuffle. Inform users not to provide int arg.
http://hg.python.org/cpython/rev/82bdd5fc7a71
New changeset 4782faf29480 by Senthil Kumaran in branch '3.3':
Improve the docstring of random.shuffle. Inform users not to provide int arg.
http://hg.python.org/cpython/rev/4782faf29480
New changeset 15096b93ae5a by Senthil Kumaran in branch 'default':
merge from 3.3
http://hg.python.org/cpython/rev/15096b93ae5a 

This is fixed in all versions. Thank you!

I would propose a leading underscore for these methods; they should make it clear to the user that the parameter is meant to be "private".
With this line in the docstring, I would wonder "Why is the argument there in the first place?!" (because most people don't know about the optimization technique).

> Georg Brandl added the comment:
>
> I would propose a leading underscore for these methods; they should make it clear to the user that the parameter is meant to be "private".
+1 to this proposal.
This style is present with randrange, randbelow and shuffle.
> With this line in the docstring, I would wonder "Why is the argument there in the first place?!" (because most people don't know about the optimization technique).
This style has been present since the introduction of these functions.
One attempt to clarify / cleanup shuffle here is: 29430:b80a22250e4f
Georg : I am unaware of the optimization technique you refer to as
well, it will helpful if you could point to any resource.

[Senthil Kumaran]
> I am unaware of the optimization technique you refer to as
> well, it will helpful if you could point to any resource.
It's an old trick since the very first Pythons: global lookups are
much slower than local lookups (the difference between the LOAD_GLOBAL
and LOAD_FAST opcodes in CPython). Putting:
 ..., _fast=slow, ...
in an argument list means we endure the slow lookup (of `slow`) only
once, when the function is first defined. When the function is
_called_, that binding is available via the local (much faster lookup)
variable `_fast`.
Purely a speed trick, but can make a real difference in very heavily
used functions.
And it's a Good Idea to put a leading underscore on such arguments.
It's never intended that users pass values for them.

Tim Peters added the comment:
> ..., _fast=slow, ...
>
> in an argument list means we endure the slow lookup (of `slow`) only
> once, when the function is first defined. When the function is
> _called_, that binding is available via the local (much faster lookup)
> variable `_fast`.
>
> Purely a speed trick, but can make a real difference in very heavily
> used functions.
Thanks for the answer, Tim. Interesting optimization.

Attaching a patch after changing int=int to _int = int and improving the docstring. Please review the changes to the docstring and see if it will be helpful.

I wouldn't add info about the optimization in the docstring.
In _randbelow() I think you missed a call to int().
For _randbelow(), all arguments after "int" are non-public ones.
(_randbelow as a private function wouldn't necessarily need the change, but it's good for consistency.)
Final decision should be made by Raymond.

Thanks for catching the mistake at _randbelow. Updated patch to fix that and removed the explanation in the docstring. Not sure if _randbelow should changed (fully) or not at all. Leaving the change only with _int.
Will wait for Raymond's review.

I think the "Do not supply the 'int' argument" covers it well enough. This code has been around for a very long time and isn't causing any problems.

Hi Raymond, Ezio provided some comments on improvements to the patch. Do you mind if Ezio or I take over task of improvement. Not cause problems != no need to improve.
TIA.

I don't think there is an actual problem here to be solved.

I'm old, but I liked the docs better when they didn't mention "the int argument" at all. The "int=int" - or "_int=int" - argument is a CPython implementation detail. It has nothing to do with the API. And _of course_ users shouldn't mess with implementation details. 99.9+% will never notice the argument is there, and the fraction that do notice should infer that they shouldn't mess with it from that it's _not_ documented.

I probably wouldn't have noticed it except I was working more intensely
with the different random methods and saw that randrange had the note about
not supplying the 'int' argument and shuffle (though it had the same sort
of argument) did *not* have the comment. So that raised the issue for me.
Proabably the best thing would be do remove the comment from randrange and
make sure that the not-to-mess-with args are made private with the
underscore.
/c

New changeset b1e94e332ec8 by Raymond Hettinger in branch '2.7':
Issue 14927: Minor clean-up of function parameters in random().
http://hg.python.org/cpython/rev/b1e94e332ec8 

Py3.3: http://hg.python.org/cpython/rev/0899960835f5
Py3.4: http://hg.python.org/cpython/rev/8494d2c8ef54 

In 3.3 and 3.4 I would just change the shuffle arg from `int=int` to
`_int=int` and delete the comment in docstring regarding not supplying the
value. (In both you *removed* the argument and internally added `_int=int`.)
Note that (as far as I can see) in 3.3 you didn't remove the comment in the
docstring of shuffle like you did in 3.4
/c
On Sat, Oct 5, 2013 at 11:54 PM, Raymond Hettinger
<report@bugs.python.org>wrote:
>
> Raymond Hettinger added the comment:
>
> Py3.3: http://hg.python.org/cpython/rev/0899960835f5
> Py3.4: http://hg.python.org/cpython/rev/8494d2c8ef54
>
> ----------
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <http://bugs.python.org/issue14927>
> _______________________________________
>

New changeset 50ea4dccb03e by Raymond Hettinger in branch '3.3':
Issue 14927: Remove a docstring line that is no longer applicable.
http://hg.python.org/cpython/rev/50ea4dccb03e 

Christopher, this tracker item needs to die. It is wasting everyone's time (and churning code) over nothing.
FYI, I moved the _int=int for shuffle inside the function because the assignment was outside of the inner loop, so we weren't getting any real benefit.

On Sun, Oct 6, 2013 at 12:14 AM, Raymond Hettinger
<report@bugs.python.org>wrote:
>
> Raymond Hettinger added the comment:
>
> Christopher, this tracker item needs to die. It is wasting everyone's
> time (and churning code) over nothing.
>
>
but it's not quite dead yet...
> FYI, I moved the _int=int for shuffle inside the function because the
> assignment was outside of the inner loop, so we weren't getting any real
> benefit.
>
but cf Tim's comment regarding the advantage of leaving it in the arg list
so that the lookup is fast:
[Senthil Kumaran]
> I am unaware of the optimization technique you refer to as
> well, it will helpful if you could point to any resource.
It's an old trick since the very first Pythons: global lookups are
much slower than local lookups (the difference between the LOAD_GLOBAL
and LOAD_FAST opcodes in CPython). Putting:
 ..., _fast=slow, ...
in an argument list means we endure the slow lookup (of `slow`) only
once, when the function is first defined. When the function is
_called_, that binding is available via the local (much faster lookup)
variable `_fast`.
Purely a speed trick, but can make a real difference in very heavily
used functions.
----------
So by removing it from the arg list you have perhaps caused a regression in
performance of shuffle. Other than that, everything looks fine to me.
Thanks,
 Chris