classification
Title: Use Random.choices in tempfile
Type: performance Stage: resolved
Components: Versions: Python 3.8
process
Status: closed Resolution: wont fix
Dependencies: Superseder:
Assigned To: Nosy List: inada.naoki, rhettinger, serhiy.storchaka, wolma
Priority: normal Keywords: patch

Created on 2018-04-05 08:50 by wolma, last changed 2019-04-08 08:20 by inada.naoki. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 6383 closed wolma, 2018-04-05 08:53
Messages (8)
msg314969 - (view) Author: Wolfgang Maier (wolma) * Date: 2018-04-05 08:50
A rather trivial change: tempfile._RandomNameSequence could make use of
random.Random.choices introduced in 3.6.
IMO, the suggested change would give clearer and also faster code.
It also updates the docstring of the class, which currently talks about
a six-character string, when an eight-character string gets returned.
msg314974 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-04-05 10:01
See issue24567. It affects Random.choices(), but not Random.choice() because the latter uses getrandbits().
msg314975 - (view) Author: Inada Naoki (inada.naoki) * (Python committer) Date: 2018-04-05 10:18
@wolma Would you split your PR?  One for fix docstring and one for using Random.choices.

Since former is document bug, I want to backport it. (while _RandomNameSequence is private)
msg314979 - (view) Author: Wolfgang Maier (wolma) * Date: 2018-04-05 11:47
@serhiy as I understand issue 33228, the double rounding problem potentially causing an IndexError can only affect choices() if the len of the sequence to choose from is greater than 2049, but the string in question here is only 37 characters long.
Alternatively, choices may fail with certain weights (https://bugs.python.org/msg275594), but _RandomNameSequence is not using weights.
msg314980 - (view) Author: Wolfgang Maier (wolma) * Date: 2018-04-05 11:48
sorry, should have been issue 24567, of course.
msg314981 - (view) Author: Wolfgang Maier (wolma) * Date: 2018-04-05 11:56
Actually, in Python2.7 random.choice is implemented with the same susceptibility to the rounding bug as Python3's choices, still nobody ever reported a tempfile IndexError problem (I guess).
msg314994 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2018-04-05 16:41
-0 on making anything other than the doc change.  For the most part, we avoid sweeping through the standard library to substitute-in new features and instead have a preference for code stability rather than code churn.

FWIW, the main rationale for introducing choices() was to support weighting.  Existing code that calls choice() multiple times is still correct and clear.
msg339598 - (view) Author: Inada Naoki (inada.naoki) * (Python committer) Date: 2019-04-08 08:20
I'm +0 too.  Since there are no +1 from core dev for a long time,
I close this issue for now.
History
Date User Action Args
2019-04-08 08:20:38inada.naokisetstatus: open -> closed
resolution: wont fix
messages: + msg339598

stage: patch review -> resolved
2018-04-05 16:41:07rhettingersetnosy: + rhettinger
messages: + msg314994
2018-04-05 11:56:46wolmasetmessages: + msg314981
2018-04-05 11:48:27wolmasetmessages: + msg314980
2018-04-05 11:47:19wolmasetmessages: + msg314979
2018-04-05 10:18:33inada.naokisetnosy: + inada.naoki
messages: + msg314975
2018-04-05 10:01:15serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg314974
2018-04-05 08:53:02wolmasetkeywords: + patch
stage: patch review
pull_requests: + pull_request6093
2018-04-05 08:50:52wolmacreate