bpo-40282: Allow random.getrandbits(0) #19539
Conversation
| cum |= v | ||
| cpl_cum |= all_bits ^ v | ||
| self.assertEqual(cum, all_bits) | ||
| self.assertEqual(cpl_cum, all_bits) |
There was a problem hiding this comment.
Note I've enhanced this test a bit to check that all bits take both values 0 and 1 accross 100 draws.
| @@ -261,6 +261,8 @@ def randint(self, a, b): | |||
| def _randbelow_with_getrandbits(self, n): | |||
| "Return a random int in the range [0,n). Raises ValueError if n==0." | |||
|
|
|||
| if n == 0: | |||
| raise ValueError("Boundary cannot be zero") | |||
| getrandbits = self.getrandbits | |||
| k = n.bit_length() # don't use (n-1) here because n can be 1 | |||
There was a problem hiding this comment.
We can use (n-1) now.
| k = n.bit_length() # don't use (n-1) here because n can be 1 | |
| k = (n-1).bit_length() |
There was a problem hiding this comment.
I think we want to ensure that the values produced for a given seed don't change, so we probably can't change this anymore.
Lib/random.py
Outdated
| if n == 0: | ||
| raise ValueError("Boundary cannot be zero") |
There was a problem hiding this comment.
This is the same error as in the other implementation (the one without getrandbits).
There was a problem hiding this comment.
Seems this check causes a performance regression.
There was a problem hiding this comment.
Is there any situation where a user could specifically encounter the above ValueError from the public methods? If not, I think an assertion would make more sense here; seeing as it's a private member that's only being used from within the random module. IMO, the same applies to _randbelow_without_getrandbits().
Also, how significant is the performance regression @serhiy-storchaka?
|
This makes shuffle() about 6% slower: python -m timeit -r11 -s 'from random import shuffle' -s 's=list(range(1000))' 'shuffle(s)' |
|
If the subclass implements |
Co-Authored-By: Serhiy Storchaka <storchaka@gmail.com>
Co-Authored-By: Victor Stinner <vstinner@python.org>
|
I rebased and applied two trivial changes from reviewing. Will merge if CI is green. Thanks everyone! |
I created bpo-40346: "Redesign random.Random class inheritance". |
* master: (1985 commits) bpo-40179: Fix translation of #elif in Argument Clinic (pythonGH-19364) bpo-35967: Skip test with `uname -p` on Android (pythonGH-19577) bpo-40257: Improve help for the typing module (pythonGH-19546) Fix two typos in multiprocessing (pythonGH-19571) bpo-40286: Use random.randbytes() in tests (pythonGH-19575) bpo-40286: Makes simpler the relation between randbytes() and getrandbits() (pythonGH-19574) bpo-39894: Route calls from pathlib.Path.samefile() to os.stat() via the path accessor (pythonGH-18836) bpo-39897: Remove needless `Path(self.parent)` call, which makes `is_mount()` misbehave in `Path` subclasses. (pythonGH-18839) bpo-40282: Allow random.getrandbits(0) (pythonGH-19539) bpo-40302: UTF-32 encoder SWAB4() macro use a|b rather than a+b (pythonGH-19572) bpo-40302: Replace PY_INT64_T with int64_t (pythonGH-19573) bpo-40286: Add randbytes() method to random.Random (pythonGH-19527) bpo-39901: Move `pathlib.Path.owner()` and `group()` implementations into the path accessor. (pythonGH-18844) bpo-40300: Allow empty logging.Formatter.default_msec_format. (pythonGH-19551) bpo-40302: Add pycore_byteswap.h header file (pythonGH-19552) bpo-40287: Fix SpooledTemporaryFile.seek() return value (pythonGH-19540) Minor modernization and readability improvement to the tokenizer example (pythonGH-19558) bpo-40294: Fix _asyncio when module is loaded/unloaded multiple times (pythonGH-19542) Fix parameter names in assertIn() docs (pythonGH-18829) bpo-39793: use the same domain on make_msgid tests (python#18698) ...
https://bugs.python.org/issue40282