[issue42772] randrange() mishandles step when stop is None

[Raymond Hettinger]

Change by Raymond Hettinger :


--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

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

[issue42772] randrange() mishandles step when stop is None

[Raymond Hettinger]

Raymond Hettinger  added the comment:


New changeset 768fa145cfec2a0599802b74fc31d2bc2812ed96 by Raymond Hettinger in 
branch 'master':
bpo-42772: Step argument ignored when stop is None. (GH-24018)
https://github.com/python/cpython/commit/768fa145cfec2a0599802b74fc31d2bc2812ed96


--

___
Python tracker 

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

[issue42772] randrange() mishandles step when stop is None

[Raymond Hettinger]

Change by Raymond Hettinger :


--
assignee:  -> rhettinger

___
Python tracker 

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

[issue42772] randrange() mishandles step when stop is None

[Raymond Hettinger]

Raymond Hettinger  added the comment:

Upon further reflection, I think these cases should raise a TypeError because 
the meaning is ambiguous:

randrange(1000, step=10) # Could only mean start=0 stop=1000 step=10

but that conflicts with:

randrange(1000, None, 100)  # Has no reasonable interpretation

For comparison, this currently raises a TypeError because None isn't a sensible 
argument for stop:

range(1000, None, 100)

--

___
Python tracker 

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

[issue42772] randrange() mishandles step when stop is None

[Raymond Hettinger]

Change by Raymond Hettinger :


--
keywords: +patch
pull_requests: +22858
stage:  -> patch review
pull_request: https://github.com/python/cpython/pull/24018

___
Python tracker 

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

[issue42772] randrange() mishandles step when stop is None

[Raymond Hettinger]

Raymond Hettinger  added the comment:

> Should I restore that optimization in issue37319?

Yes, but to fix the bug it needs to occur earlier in the code (currently line 
314):

if stop is None and step is _ONE: <-- Line 314
   ^  <-- Add this   

And again later:
 if istep == 1 and width > 0:   <-- Line 348
 ^-- change to _ONE

As a standalone optimization, it wasn't worth it, but as a way to fix the bug 
without a regression, it is reasonable.

--

___
Python tracker 

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

[issue42772] randrange() mishandles step when stop is None

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

> Another solution is to use an identity test for the step argument

Should I restore that optimization in issue37319?

--

___
Python tracker 

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

[issue42772] randrange() mishandles step when stop is None

[Raymond Hettinger]

Raymond Hettinger  added the comment:

Another solution is to use an identity test for the step argument

_one = 1

class Random:
def randrange(start, stop=None, step=_one):
...
if stop is None and step is _one:
if istart > 0:
return self._randbelow(istart)

This has the advantage of keeping the API unchanged while still keeping the 
fast path fast.

--

___
Python tracker 

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

[issue42772] randrange() mishandles step when stop is None

[Raymond Hettinger]

Raymond Hettinger  added the comment:

Note, we can't actually use "self.choice(range(*args))" because the underlying 
len() call can Overflow.  If it does, the components need to be computed 
manually.

--

___
Python tracker 

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

[issue42772] randrange() mishandles step when stop is None

[Raymond Hettinger]

Change by Raymond Hettinger :


--
nosy: +tim.peters

___
Python tracker 

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

[issue42772] randrange() mishandles step when stop is None

[Raymond Hettinger]

Raymond Hettinger  added the comment:

One fix is to move up the code for converting step to istep and then modify the 
early out test to:  

if stop is None and istep == 1:
if istart > 0:
return self._randbelow(istart)

That would have the downside of slowing down the common case.

Another possibility is changing the default step to a sentinel object and just 
testing for that:

if stop is None and step is sentinel:
if istart > 0:
return self._randbelow(istart)

That would be user visible in the function signature but it would run fast.

We could bite the bullet and fully harmonize the randrange() signature with 
range().  That would entail switching to *args and no longer accepting keyword 
arguments:

 def randrange(self, /, *args):
 "Choose random item from range(stop) or range(start, stop[, step])."
 return self.choice(range(*args))

This would most closely match user expectations but is potentially a breaking 
change for existing code that uses keyword arguments.  Such code probably 
exists but is probably not common.

For speed, the actual implementation could still have fast paths for common 
cases.

For help() and tooltips, we could add a __text_signature__ to cover-up the 
*args.  However, signature objects currently aren't capable of describing 
range().  The best we could do is:

randrange.__text_signature__ = '($self, start, stop, step, /)'

That would give help() that looks like this:

>>> help(randrange)
Help on method randrange in module Random:

randrange(start, stop, step, /) method of __main__.R instance
Choose random item from range(stop) or range(start, stop[, step]).

--

___
Python tracker 

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

[issue42772] randrange() mishandles step when stop is None

[Raymond Hettinger]

Change by Raymond Hettinger :


--
assignee: rhettinger -> 
versions: +Python 3.9

___
Python tracker 

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

[issue42772] randrange() mishandles step when stop is None

[Raymond Hettinger]

Change by Raymond Hettinger :


--
assignee:  -> rhettinger

___
Python tracker 

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

[issue42772] randrange() mishandles step when stop is None

[Raymond Hettinger]

New submission from Raymond Hettinger :

When stop is None, the step argument is ignored:

>>> randrange(1000, None, 100)
651
>>> randrange(1000, step=100)
673

--
components: Library (Lib)
messages: 383919
nosy: rhettinger, serhiy.storchaka
priority: normal
severity: normal
status: open
title: randrange() mishandles step when stop is None
type: behavior
versions: Python 3.10

___
Python tracker 

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