Issue 42222: Modernize integer test/conversion in randrange() (original) (raw)

Created on 2020-10-31 17:27 by rhettinger, last changed 2022-04-11 14:59 by admin. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 23064 merged rhettinger,2020-10-31 17:27
PR 28983 merged rhettinger,2021-10-15 16:17
PR 29021 merged serhiy.storchaka,2021-10-18 08:14
Messages (16)
msg380082 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2020-10-31 17:27
Move the int(x)==x test and conversion into the C code for operator.index().
msg380083 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2020-10-31 17:34
And, if we were willing to correct the exception type from ValueError to TypeError, the code could be made simpler, faster, and more in line with user expectations.
msg380084 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2020-10-31 17:54
I had forgotten. It looks like float arguments were allowed: >>> randrange(10.0, 20.0, 2.0) 16 Is this worth going through a deprecation cycle to get the code cleaned-up or should we live with it as is?
msg380085 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2020-10-31 18:09
It changes the behavior. Currently randrange(10.0) works, but with PR 23064 it would fail. See with a ready PR for increasing coverage for the random module. If it would accepted, some tests would fail with PR 23064. If you want to deprecate accepting float arguments, there was with a ready PR. These propositions were rejected by you. Have you reconsidered your decision?
msg380086 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2020-10-31 18:19
> These propositions were rejected by you. > Have you reconsidered your decision? I was reluctant to break any existing code. Now, I'm unsure and am inclined to harmonize it with range(). What do you think? Should we have ever supported float arguments for an integer domain function?
msg380089 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2020-10-31 19:03
I think and always thought that integer domain functions should not accept non-integer arguments even with integer value. This is why I submitted numerous patches for deprecating and finally removing support of non-integer arguments in most of integer domain functions. C implemented functions which use PyArg_Parse("i") or PyLong_AsLong() for parsing arguments use now index() instead of int(). They emit a deprecation warning for non-integers in 3.8 and 3.9 and raise type error since 3.10. math.factorial() emits a warning only in 3.9. Issue37319 (sorry, I wrote incorrect issue number in ) was initially opened for 3.9, so we could convert warnings into errors in 3.10 or 3.11. Currently randrange(1e25) can return value larger than 10**25, because int(1e25) == 10000000000000000905969664 > 10**25.
msg380096 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2020-10-31 20:25
User feedback concur with making the change: https://twitter.com/raymondh/status/1322607969754775552
msg380099 - (view) Author: Vedran Čačić (veky) * Date: 2020-10-31 21:11
Yes, the ability to write randrange(1e9) is sometimes nice. And the fact that it might give the number outside the intended range with probability 1e-17 is not really an important argument (people have bad intuitions about very small probabilities). But if we intend to be consistent with range, then of course this must go.
msg380111 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2020-11-01 01:39
10**9 isn't much harder than 10E9 ;-)
msg380498 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2020-11-07 10:00
To me, ValueError("non-integer arg 1 for randrange()") (ValueError('bad type') is a bit painful to read. We do sometime fix such bugs, when not documented, in future releases. Current the doc, "Return a randomly selected element from range(start, stop, step). This is equivalent to choice(range(start, stop, step))", implies that both accept the same values, which most would expect anyway from the names. Being selectively 'generous' in what is accepted is confusing. For the future: both range and math.factorial raise TypeError: 'float' object cannot be interpreted as an integer The consistency is nice. randrange should say the same after deprecation.
msg380499 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2020-11-07 10:11
To put what I said another way: both items are mental paper cuts and I see benefit to both coredevs and users in getting rid of them. That is not to say 'no cost', but that there is a real benefit to be balanced against the real cost.
msg383776 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2020-12-25 22:38
There is another randrange() oddity. If stop is None, the step argument is ignored: >>> randrange(100, stop=None, step=10) 4 If we want to fully harmonize with range(), then randrange() should only accept positional arguments and should not allow None for the stop argument. That would leave the unoptimized implementation equivalent to: def randrange(self, /, *args): return self.choice(range(*args)) The actual implementation can retain its fast paths and have a nicer looking signature perhaps using __text_signature__.
msg383916 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2020-12-28 19:10
New changeset a9621bb301dba44494e81edc00e3a3b62c96af26 by Raymond Hettinger in branch 'master': bpo-42222: Modernize integer test/conversion in randrange() (#23064) https://github.com/python/cpython/commit/a9621bb301dba44494e81edc00e3a3b62c96af26
msg404088 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2021-10-16 15:16
New changeset 5afa0a411243210a30526c7459a0ccff5cb88494 by Raymond Hettinger in branch 'main': bpo-42222: Remove deprecated support for non-integer values (GH-28983) https://github.com/python/cpython/commit/5afa0a411243210a30526c7459a0ccff5cb88494
msg404327 - (view) Author: Łukasz Langa (lukasz.langa) * (Python committer) Date: 2021-10-19 18:40
New changeset 574241632bd19e56ed488ee4d8999aefc6a8d7cd by Serhiy Storchaka in branch 'main': bpo-42222: Improve tests for invalid argument types in randrange() (GH-29021) https://github.com/python/cpython/commit/574241632bd19e56ed488ee4d8999aefc6a8d7cd
msg412435 - (view) Author: Petr Viktorin (petr.viktorin) * (Python committer) Date: 2022-02-03 10:41
Since this is a user-visible change in 3.11, could you add a What's New entry?
History
Date User Action Args
2022-04-11 14:59:37 admin set github: 86388
2022-02-03 10:41:29 petr.viktorin set nosy: + petr.viktorinmessages: +
2021-10-19 18:45:42 lukasz.langa set versions: + Python 3.11, - Python 3.10
2021-10-19 18:40:37 lukasz.langa set nosy: + lukasz.langamessages: +
2021-10-18 08:14:50 serhiy.storchaka set pull_requests: + <pull%5Frequest27294>
2021-10-16 15:16:58 rhettinger set messages: +
2021-10-15 16:17:44 rhettinger set pull_requests: + <pull%5Frequest27270>
2020-12-28 19:11:34 rhettinger set status: open -> closedresolution: fixedstage: patch review -> resolved
2020-12-28 19:10:53 rhettinger set messages: +
2020-12-25 22:38:51 rhettinger set messages: +
2020-12-24 09🔞46 rhettinger set assignee: rhettinger
2020-11-07 10:11:45 terry.reedy set messages: +
2020-11-07 10:00:59 terry.reedy set nosy: + terry.reedymessages: +
2020-11-01 01:39:33 rhettinger set messages: +
2020-10-31 21:11:58 veky set nosy: + vekymessages: +
2020-10-31 20:25:04 rhettinger set messages: +
2020-10-31 19:03:18 serhiy.storchaka set messages: +
2020-10-31 18:19:03 rhettinger set messages: +
2020-10-31 18:09:28 serhiy.storchaka set messages: +
2020-10-31 17:54:30 rhettinger set messages: +
2020-10-31 17:34:52 rhettinger set messages: +
2020-10-31 17:27:39 rhettinger set keywords: + patchstage: patch reviewpull_requests: + <pull%5Frequest21983>
2020-10-31 17:27:12 rhettinger create