From f572b535baec4f84e1b1407ceb326133642b4bff Mon Sep 17 00:00:00 2001 From: Antoine Pitrou Date: Wed, 15 Apr 2020 10:22:00 +0200 Subject: [PATCH 1/7] bpo-40282: Allow random.getrandbits(0) --- Doc/library/random.rst | 3 + Lib/random.py | 13 ++-- Lib/test/test_random.py | 68 ++++++++----------- .../2020-04-15-10-23-52.bpo-40282.rIYJmu.rst | 1 + Modules/_randommodule.c | 8 ++- 5 files changed, 43 insertions(+), 50 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2020-04-15-10-23-52.bpo-40282.rIYJmu.rst diff --git a/Doc/library/random.rst b/Doc/library/random.rst index 1eb39bbda42e81..179adb033b9fb3 100644 --- a/Doc/library/random.rst +++ b/Doc/library/random.rst @@ -111,6 +111,9 @@ Bookkeeping functions as an optional part of the API. When available, :meth:`getrandbits` enables :meth:`randrange` to handle arbitrarily large ranges. + .. versionchanged:: 3.9 + This method now accepts 0 for *k*. + Functions for integers ---------------------- diff --git a/Lib/random.py b/Lib/random.py index e24737d4508a8f..e7d109f2219f55 100644 --- a/Lib/random.py +++ b/Lib/random.py @@ -295,11 +295,10 @@ def _randbelow_without_getrandbits(self, n, int=int, maxsize=1< x. Generates an int with k random bits.""" - if k <= 0: - raise ValueError('number of bits must be greater than zero') + if k < 0: + raise ValueError('number of bits must be positive') numbytes = (k + 7) // 8 # bits / 8 and rounded up x = int.from_bytes(_urandom(numbytes), 'big') return x >> (numbytes * 8 - k) # trim excess bits diff --git a/Lib/test/test_random.py b/Lib/test/test_random.py index 548af706dbee22..0e834e8f5cc5e6 100644 --- a/Lib/test/test_random.py +++ b/Lib/test/test_random.py @@ -263,6 +263,30 @@ def test_gauss(self): self.assertEqual(x1, x2) self.assertEqual(y1, y2) + def test_getrandbits(self): + # Verify ranges + for k in range(0, 1000): + self.assertTrue(0 <= self.gen.getrandbits(k) < 2**k) + + # Verify all bits active + getbits = self.gen.getrandbits + for span in [1, 2, 3, 4, 31, 32, 32, 52, 53, 54, 119, 127, 128, 129]: + all_bits = 2**span-1 + cum = 0 + cpl_cum = 0 + for i in range(100): + v = getbits(span) + cum |= v + cpl_cum |= all_bits ^ v + self.assertEqual(cum, all_bits) + self.assertEqual(cpl_cum, all_bits) + + # Verify argument checking + self.assertRaises(TypeError, self.gen.getrandbits) + self.assertRaises(TypeError, self.gen.getrandbits, 1, 2) + self.assertRaises(ValueError, self.gen.getrandbits, -1) + self.assertRaises(TypeError, self.gen.getrandbits, 10.1) + def test_pickling(self): for proto in range(pickle.HIGHEST_PROTOCOL + 1): state = pickle.dumps(self.gen, proto) @@ -374,26 +398,6 @@ def test_randrange_errors(self): raises(0, 42, 0) raises(0, 42, 3.14159) - def test_genrandbits(self): - # Verify ranges - for k in range(1, 1000): - self.assertTrue(0 <= self.gen.getrandbits(k) < 2**k) - - # Verify all bits active - getbits = self.gen.getrandbits - for span in [1, 2, 3, 4, 31, 32, 32, 52, 53, 54, 119, 127, 128, 129]: - cum = 0 - for i in range(100): - cum |= getbits(span) - self.assertEqual(cum, 2**span-1) - - # Verify argument checking - self.assertRaises(TypeError, self.gen.getrandbits) - self.assertRaises(TypeError, self.gen.getrandbits, 1, 2) - self.assertRaises(ValueError, self.gen.getrandbits, 0) - self.assertRaises(ValueError, self.gen.getrandbits, -1) - self.assertRaises(TypeError, self.gen.getrandbits, 10.1) - def test_randbelow_logic(self, _log=log, int=int): # check bitcount transition points: 2**i and 2**(i+1)-1 # show that: k = int(1.001 + _log(n, 2)) @@ -613,34 +617,18 @@ def test_rangelimits(self): self.assertEqual(set(range(start,stop)), set([self.gen.randrange(start,stop) for i in range(100)])) - def test_genrandbits(self): + def test_getrandbits(self): + super().test_getrandbits() + # Verify cross-platform repeatability self.gen.seed(1234567) self.assertEqual(self.gen.getrandbits(100), 97904845777343510404718956115) - # Verify ranges - for k in range(1, 1000): - self.assertTrue(0 <= self.gen.getrandbits(k) < 2**k) - - # Verify all bits active - getbits = self.gen.getrandbits - for span in [1, 2, 3, 4, 31, 32, 32, 52, 53, 54, 119, 127, 128, 129]: - cum = 0 - for i in range(100): - cum |= getbits(span) - self.assertEqual(cum, 2**span-1) - - # Verify argument checking - self.assertRaises(TypeError, self.gen.getrandbits) - self.assertRaises(TypeError, self.gen.getrandbits, 'a') - self.assertRaises(TypeError, self.gen.getrandbits, 1, 2) - self.assertRaises(ValueError, self.gen.getrandbits, 0) - self.assertRaises(ValueError, self.gen.getrandbits, -1) def test_randrange_uses_getrandbits(self): # Verify use of getrandbits by randrange # Use same seed as in the cross-platform repeatability test - # in test_genrandbits above. + # in test_getrandbits above. self.gen.seed(1234567) # If randrange uses getrandbits, it should pick getrandbits(100) # when called with a 100-bits stop argument. diff --git a/Misc/NEWS.d/next/Library/2020-04-15-10-23-52.bpo-40282.rIYJmu.rst b/Misc/NEWS.d/next/Library/2020-04-15-10-23-52.bpo-40282.rIYJmu.rst new file mode 100644 index 00000000000000..699282a7fb59cb --- /dev/null +++ b/Misc/NEWS.d/next/Library/2020-04-15-10-23-52.bpo-40282.rIYJmu.rst @@ -0,0 +1 @@ +Allow ``random.getrandbits(0)`` to succeed and to return 0. diff --git a/Modules/_randommodule.c b/Modules/_randommodule.c index 90762758f93115..cb1c859ce89fab 100644 --- a/Modules/_randommodule.c +++ b/Modules/_randommodule.c @@ -474,12 +474,14 @@ _random_Random_getrandbits_impl(RandomObject *self, int k) uint32_t *wordarray; PyObject *result; - if (k <= 0) { - PyErr_SetString(PyExc_ValueError, - "number of bits must be greater than zero"); + if (k < 0) { + PyErr_SetString(PyExc_ValueError, "number of bits must be positive"); return NULL; } + if (k == 0) + return PyLong_FromLong(0); + if (k <= 32) /* Fast path */ return PyLong_FromUnsignedLong(genrand_int32(self) >> (32 - k)); From 3153f1a231b298042fd2a187caeebdea90dace07 Mon Sep 17 00:00:00 2001 From: Antoine Pitrou Date: Wed, 15 Apr 2020 10:51:29 +0200 Subject: [PATCH 2/7] Update Doc/library/random.rst Co-Authored-By: Serhiy Storchaka --- Doc/library/random.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Doc/library/random.rst b/Doc/library/random.rst index 179adb033b9fb3..b50e6a6627a6b7 100644 --- a/Doc/library/random.rst +++ b/Doc/library/random.rst @@ -112,7 +112,7 @@ Bookkeeping functions :meth:`randrange` to handle arbitrarily large ranges. .. versionchanged:: 3.9 - This method now accepts 0 for *k*. + This method now accepts ``0`` for *k*. Functions for integers From 9cac35f8ee481aa0c8ff0c72140bdee50cc8790f Mon Sep 17 00:00:00 2001 From: Antoine Pitrou Date: Wed, 15 Apr 2020 10:55:50 +0200 Subject: [PATCH 3/7] Ensure _randbelow implementations raise on 0 input. --- Lib/random.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Lib/random.py b/Lib/random.py index e7d109f2219f55..e3ed75fcbb5c5e 100644 --- a/Lib/random.py +++ b/Lib/random.py @@ -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 r = getrandbits(k) # 0 <= r < 2**k From b01be919a860d8abae074541161606092289e601 Mon Sep 17 00:00:00 2001 From: Antoine Pitrou Date: Wed, 15 Apr 2020 11:39:18 +0200 Subject: [PATCH 4/7] s/positive/non-negative/ --- Lib/random.py | 2 +- Modules/_randommodule.c | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/Lib/random.py b/Lib/random.py index e3ed75fcbb5c5e..ab68103a5db1a6 100644 --- a/Lib/random.py +++ b/Lib/random.py @@ -735,7 +735,7 @@ def random(self): def getrandbits(self, k): """getrandbits(k) -> x. Generates an int with k random bits.""" if k < 0: - raise ValueError('number of bits must be positive') + raise ValueError('number of bits must be non-negative') numbytes = (k + 7) // 8 # bits / 8 and rounded up x = int.from_bytes(_urandom(numbytes), 'big') return x >> (numbytes * 8 - k) # trim excess bits diff --git a/Modules/_randommodule.c b/Modules/_randommodule.c index cb1c859ce89fab..bd27c5ed357c8e 100644 --- a/Modules/_randommodule.c +++ b/Modules/_randommodule.c @@ -475,7 +475,8 @@ _random_Random_getrandbits_impl(RandomObject *self, int k) PyObject *result; if (k < 0) { - PyErr_SetString(PyExc_ValueError, "number of bits must be positive"); + PyErr_SetString(PyExc_ValueError, + "number of bits must be non-negative"); return NULL; } From e7c2e99ba3bef096a566b8e21d6155f8d9ed4700 Mon Sep 17 00:00:00 2001 From: Antoine Pitrou Date: Wed, 15 Apr 2020 14:08:43 +0200 Subject: [PATCH 5/7] Revert changes to choice() --- Lib/random.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/Lib/random.py b/Lib/random.py index ab68103a5db1a6..599cbe9e536fea 100644 --- a/Lib/random.py +++ b/Lib/random.py @@ -297,10 +297,11 @@ def _randbelow_without_getrandbits(self, n, int=int, maxsize=1< Date: Wed, 15 Apr 2020 18:37:08 +0200 Subject: [PATCH 6/7] Update Lib/test/test_random.py Co-Authored-By: Victor Stinner --- Lib/test/test_random.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_random.py b/Lib/test/test_random.py index 0e834e8f5cc5e6..8a41fb2ca889dd 100644 --- a/Lib/test/test_random.py +++ b/Lib/test/test_random.py @@ -265,8 +265,9 @@ def test_gauss(self): def test_getrandbits(self): # Verify ranges - for k in range(0, 1000): + for k in range(1, 1000): self.assertTrue(0 <= self.gen.getrandbits(k) < 2**k) + self.assertEqual(self.gen.getrandbits(0), 0) # Verify all bits active getbits = self.gen.getrandbits From c4673f025739bf186b00cd93183704230b1bf1a2 Mon Sep 17 00:00:00 2001 From: Antoine Pitrou Date: Fri, 17 Apr 2020 18:24:31 +0200 Subject: [PATCH 7/7] Apply review comments --- Doc/library/random.rst | 2 +- Lib/random.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Doc/library/random.rst b/Doc/library/random.rst index b50e6a6627a6b7..daeff664563a76 100644 --- a/Doc/library/random.rst +++ b/Doc/library/random.rst @@ -112,7 +112,7 @@ Bookkeeping functions :meth:`randrange` to handle arbitrarily large ranges. .. versionchanged:: 3.9 - This method now accepts ``0`` for *k*. + This method now accepts zero for *k*. Functions for integers diff --git a/Lib/random.py b/Lib/random.py index 599cbe9e536fea..a15a54b25b25cc 100644 --- a/Lib/random.py +++ b/Lib/random.py @@ -261,7 +261,7 @@ 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: + if not n: raise ValueError("Boundary cannot be zero") getrandbits = self.getrandbits k = n.bit_length() # don't use (n-1) here because n can be 1