[issue14927] add "Do not supply 'int' argument" to random.shuffle docstring

[Christopher Smith]

Christopher Smith added the comment:

On Sun, Oct 6, 2013 at 12:14 AM, Raymond Hettinger
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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14927] add "Do not supply 'int' argument" to random.shuffle docstring

[Raymond Hettinger]

Raymond Hettinger added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14927] add "Do not supply 'int' argument" to random.shuffle docstring

[Roundup Robot]

Roundup Robot added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14927] add "Do not supply 'int' argument" to random.shuffle docstring

[Christopher Smith]

Christopher Smith added the comment:

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
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 
> 
> ___
>

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14927] add "Do not supply 'int' argument" to random.shuffle docstring

[Raymond Hettinger]

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 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14927] add "Do not supply 'int' argument" to random.shuffle docstring

[Roundup Robot]

Roundup Robot added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14927] add "Do not supply 'int' argument" to random.shuffle docstring

[Christopher Smith]

Christopher Smith added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14927] add "Do not supply 'int' argument" to random.shuffle docstring

[Tim Peters]

Tim Peters added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14927] add "Do not supply 'int' argument" to random.shuffle docstring

[Raymond Hettinger]

Raymond Hettinger added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14927] add "Do not supply 'int' argument" to random.shuffle docstring

[Senthil Kumaran]

Senthil Kumaran added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14927] add "Do not supply 'int' argument" to random.shuffle docstring

[Raymond Hettinger]

Raymond Hettinger added the comment:

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.

--
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14927] add "Do not supply 'int' argument" to random.shuffle docstring

[Ezio Melotti]

Changes by Ezio Melotti :


--
nosy: +ezio.melotti
versions: +Python 3.4 -Python 3.2

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14927] add "Do not supply 'int' argument" to random.shuffle docstring

[Senthil Kumaran]

Senthil Kumaran added the comment:

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.

--
Added file: http://bugs.python.org/file31789/14927-2.diff

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14927] add "Do not supply 'int' argument" to random.shuffle docstring

[Georg Brandl]

Georg Brandl added the comment:

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.

--
status: pending -> open

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14927] add "Do not supply 'int' argument" to random.shuffle docstring

[Senthil Kumaran]

Senthil Kumaran added the comment:

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.

--
status: closed -> pending
Added file: http://bugs.python.org/file31788/14927.diff

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14927] add "Do not supply 'int' argument" to random.shuffle docstring

[Senthil Kumaran]

Senthil Kumaran added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14927] add "Do not supply 'int' argument" to random.shuffle docstring

[Tim Peters]

Tim Peters added the comment:

[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.

--
nosy: +tim.peters

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14927] add "Do not supply 'int' argument" to random.shuffle docstring

[Senthil Kumaran]

Senthil Kumaran added the comment:

> 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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14927] add "Do not supply 'int' argument" to random.shuffle docstring

[Georg Brandl]

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".

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).

--
nosy: +georg.brandl

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14927] add "Do not supply 'int' argument" to random.shuffle docstring

[Senthil Kumaran]

Senthil Kumaran added the comment:

This is fixed in all versions. Thank you!

--
nosy: +orsenthil
resolution:  -> fixed
stage: needs patch -> committed/rejected
status: open -> closed
type:  -> behavior

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14927] add "Do not supply 'int' argument" to random.shuffle docstring

[Roundup Robot]

Roundup Robot added the comment:

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

--
nosy: +python-dev

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14927] add "Do not supply 'int' argument" to random.shuffle docstring

[Raymond Hettinger]

Changes by Raymond Hettinger :


--
priority: normal -> low

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14927] add "Do not supply 'int' argument" to random.shuffle docstring

[Raymond Hettinger]

Changes by Raymond Hettinger :


--
assignee:  -> rhettinger

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14927] add "Do not supply 'int' argument" to random.shuffle docstring

[Christopher Smith]

Christopher Smith  added the comment:

On Tue, Jun 12, 2012 at 1:34 AM, Michael Driscoll
 wrote:
>
> Michael Driscoll  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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14927] add "Do not supply 'int' argument" to random.shuffle docstring

[Michael Driscoll]

Michael Driscoll  added the comment:

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

--
keywords: +patch
nosy: +michael.driscoll
Added file: http://bugs.python.org/file25938/shuffle.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14927] add "Do not supply 'int' argument" to random.shuffle docstring

[Éric Araujo]

Changes by Éric Araujo :


--
keywords: +easy
nosy: +sandro.tosi
stage:  -> needs patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14927] add "Do not supply 'int' argument" to random.shuffle docstring

[Éric Araujo]

Changes by Éric Araujo :


--
nosy: +eric.araujo
title: add "Do not supply int argument" to random.shuffle docstring -> add "Do 
not supply 'int' argument" to random.shuffle docstring

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14927] add "Do not supply int argument" to random.shuffle docstring

[Christopher Smith]

Christopher Smith  added the comment:


> 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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14927] add "Do not supply int argument" to random.shuffle docstring

[Mark Dickinson]

Changes by Mark Dickinson :


--
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

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com