GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — bpo-37319: Deprecated support of non-integer arguments in random.randrange(). by serhiy-storchaka · Pull Request #19112 · python/cpython

Conversation

@serhiy-storchaka

@serhiy-storchaka serhiy-storchaka commented Mar 22, 2020 •
edited by bedevere-bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

https://bugs.python.org/issue37319

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
serhiy-storchaka added 2 commits March 22, 2020 20:44
@serhiy-storchaka
Use index() instead of int() to check args of randrange().
9f81ab8
@serhiy-storchaka
Fallaback to int().
ccff52e
@serhiy-storchaka
serhiy-storchaka requested review from mdickinson and rhettinger March 22, 2020 19:35
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label Mar 22, 2020
@bedevere-bot bedevere-bot added the awaiting core review label Mar 22, 2020
@rhettinger

rhettinger commented Mar 22, 2020

Copy link
Copy Markdown
Contributor

I don't think this should be done. It churns an API that has been stable for two decades. It doesn't solve a real problem. It may break working code. It makes the code look gross. It will complicate maintenance.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
rhettinger
rhettinger reviewed Mar 22, 2020
View reviewed changes
Comment thread Lib/random.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@alimcmaster1

alimcmaster1 commented Apr 11, 2020

Copy link
Copy Markdown

Looks like this is good to close from the discussion on https://bugs.python.org/issue37319 ?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@serhiy-storchaka serhiy-storchaka closed this Apr 11, 2020
@serhiy-storchaka
serhiy-storchaka deleted the randrange-index branch April 11, 2020 08:01
@serhiy-storchaka
serhiy-storchaka restored the randrange-index branch October 31, 2020 19:33
@serhiy-storchaka serhiy-storchaka reopened this Oct 31, 2020
@serhiy-storchaka
Merge branch 'master' into randrange-index
fb9382e
@serhiy-storchaka

serhiy-storchaka commented Oct 31, 2020

Copy link
Copy Markdown
Member Author

Reopened because @rhettinger just opened a duplicate issue.

According to the same microbenchmark as in #23064, this PR provides a 10% speed up:

$ ./python -m timeit -s 'from random import randrange' 'randrange(15)' # baseline
500000 loops, best of 5: 515 nsec per loop
$ ./python -m timeit -s 'from random import randrange' 'randrange(15)'  # new version
500000 loops, best of 5: 467 nsec per loop
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
rhettinger
rhettinger reviewed Oct 31, 2020
View reviewed changes
Comment thread Lib/random.py
except TypeError:
istep = int(step)
if istep != step:
raise ValueError("non-integer step for randrange()")

@rhettinger rhettinger Oct 31, 2020

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While we're at it, the exception type should be converted to TypeError.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@serhiy-storchaka serhiy-storchaka Oct 31, 2020

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we raise it with custom message or reraise the exception raised by index()?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
serhiy-storchaka added 3 commits October 31, 2020 22:50
@serhiy-storchaka
Remove _ONE.
fcd6549
@serhiy-storchaka
Raise TypeError instead of ValueError.
2e8cfda
@serhiy-storchaka
Merge branch 'master' into randrange-index
d2925e6
@serhiy-storchaka serhiy-storchaka mentioned this pull request Dec 24, 2020
bpo-42222: Modernize integer test/conversion in randrange() #23064
Merged
@serhiy-storchaka

serhiy-storchaka commented Dec 24, 2020

Copy link
Copy Markdown
Member Author

@rhettinger, what are your problems with this PR? I addressed your comments, and would appreciate any other suggestions.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
serhiy-storchaka added 2 commits December 24, 2020 13:52
@serhiy-storchaka
Revert "Raise TypeError instead of ValueError."
6ff20ab 
This reverts commit 2e8cfda.
@serhiy-storchaka
Update docs and emit deprecation warning for ValueError.
3ad3c36
@rhettinger

rhettinger commented Dec 28, 2020

Copy link
Copy Markdown
Contributor

I merged in PR 23064. If you want to make further modifications or improvements that would be welcome.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@serhiy-storchaka
Merge branch 'master' into randrange-index
5749575
@serhiy-storchaka
serhiy-storchaka force-pushed the randrange-index branch from 9e238e7 to 5749575 Compare December 28, 2020 22:09
@serhiy-storchaka

serhiy-storchaka commented Dec 29, 2020

Copy link
Copy Markdown
Member Author

I merged with master. Please make review.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@serhiy-storchaka
Merge branch 'master' into randrange-index
d5dec67
@serhiy-storchaka serhiy-storchaka added the skip news label Jan 25, 2021
@serhiy-storchaka
serhiy-storchaka merged commit f066bd9 into python:master Jan 25, 2021
@bedevere-bot bedevere-bot removed the awaiting core review label Jan 25, 2021
@serhiy-storchaka
serhiy-storchaka deleted the randrange-index branch January 25, 2021 21:02
adorilson pushed a commit to adorilson/cpython that referenced this pull request Mar 13, 2021
@serhiy-storchaka @adorilson
bpo-37319: Improve documentation, code and tests of randrange. (pytho…
042869c 
…nGH-19112)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rhettinger rhettinger rhettinger left review comments
@mdickinson mdickinson Awaiting requested review from mdickinson

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

6 participants

@serhiy-storchaka @rhettinger @alimcmaster1 @nenell12211 @the-knights-who-say-ni @bedevere-bot
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.