Skip to content
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-33998: randrange silently ignoring alt steps #8010

Closed
wants to merge 1 commit into from

Conversation

mr-nfamous
Copy link
Contributor

@mr-nfamous mr-nfamous commented Jun 29, 2018

randrange currently and probably has always done nothing when provided with a non-default step value. There are two solutions: make the non-default step value actually have an effect on the potential results; or, cause it to throw an exception warning users that changing the value of step is pointless without providing an explicit stopping point.

While the first solution conflicts with the behavior of range, that isn't necessarily a bad thing. Since when the 2nd positional argument is omitted the first one already get reinterpreted as the stop point, it is illogical to not expect similar behavior when an alternate step value is provided via a keyword argument.

In addition, operator.index is 4-5x faster than the int constructor which leads to a significant performance boost. Since the code looks much cleaner, the comment about its hygiene can be removed

https://bugs.python.org/issue33998

randrange currently and probably has always done nothing when provided with a non-default step value. There are two solutions: make the non-default step value actually have an effect on the potential results; or, cause it to throw an exception warning users that changing the value of step is pointless without providing an explicity stopping point.

While the first solution conflicts with the behavior of range, that isn't necessarily a bad thing. Since when the 2nd positional argument is omitted the first one  already get reinterpreted as the stop point, it is illogical to not expect similar behavior when an alternate step value is provided via a keyword argument.

In addition, operator.index is 4-5x faster than the int constructor which leads to a significant performance boost.
@korigod
Copy link

korigod commented Jul 1, 2018

I like the fix in part of step behavior but I am not certain removing checks for integers is worth it.

I would definitely open a separate issue and PR on this and let the current issue and PR to deal only with the step arg misbehavior.

@rhettinger rhettinger closed this Jul 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants