-
-
Notifications
You must be signed in to change notification settings - Fork 30.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
bpo-42222: Modernize integer test/conversion in randrange() #23064
Conversation
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.
Currently you are on the way to #19112 (which already has tests and documentation and cover corner cases).
This PR is stale because it has been open for 30 days with no activity. Remove stale label or comment or this will be closed in 5 days |
What about #19112? |
I merged in the relevant parts of #19112. The principal differences are:
|
As in #19112 initially. I changed ValueError to TypeError on your request.
I would appreciate any suggestions to change wording.
Yes, tests in #19112 cover more cases.
As well as in #19112. But this PR still has issues. It does not emit warning for |
@rhettinger: Please replace |
Moves the
int(x) == x
test and conversion into the C code for operator.index().Provides a 7% speed up:
It should become even faster when we get a zero-cost try.
It's odd that non-integers raise a ValueError. I think that should have been TypeError.
https://bugs.python.org/issue42222