From ff2db57b90c94a8797aa25357ddec7c7ef83ce52 Mon Sep 17 00:00:00 2001 From: Raymond Hettinger Date: Tue, 18 Feb 2025 19:32:00 -0600 Subject: [PATCH 1/5] gh-130285: Support zero counts in random.sample() --- Lib/random.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/Lib/random.py b/Lib/random.py index 8b9a270c429e4a..2799b9b887d52f 100644 --- a/Lib/random.py +++ b/Lib/random.py @@ -421,11 +421,14 @@ def sample(self, population, k, *, counts=None): cum_counts = list(_accumulate(counts)) if len(cum_counts) != n: raise ValueError('The number of counts does not match the population') - total = cum_counts.pop() + try: + total = cum_counts.pop() + except IndexError: + total = 0 if not isinstance(total, int): raise TypeError('Counts must be integers') - if total <= 0: - raise ValueError('Total of counts must be greater than zero') + if total < 0: + raise ValueError('Total of counts must be non-negative') selections = self.sample(range(total), k=k) bisect = _bisect return [population[bisect(cum_counts, s)] for s in selections] From 85ade390ed46a76df3a571f72c2b91b7d249ad07 Mon Sep 17 00:00:00 2001 From: Raymond Hettinger Date: Tue, 18 Feb 2025 19:47:21 -0600 Subject: [PATCH 2/5] Add test cases for zero counts or empty counts --- Lib/test/test_random.py | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_random.py b/Lib/test/test_random.py index 51f9193b269eee..96f6cc86219a5d 100644 --- a/Lib/test/test_random.py +++ b/Lib/test/test_random.py @@ -225,8 +225,6 @@ def test_sample_with_counts(self): sample(['red', 'green', 'blue'], counts=10, k=10) # counts not iterable with self.assertRaises(ValueError): sample(['red', 'green', 'blue'], counts=[-3, -7, -8], k=2) # counts are negative - with self.assertRaises(ValueError): - sample(['red', 'green', 'blue'], counts=[0, 0, 0], k=2) # counts are zero with self.assertRaises(ValueError): sample(['red', 'green'], counts=[10, 10], k=21) # population too small with self.assertRaises(ValueError): @@ -234,6 +232,20 @@ def test_sample_with_counts(self): with self.assertRaises(ValueError): sample(['red', 'green', 'blue'], counts=[1, 2, 3, 4], k=2) # too many counts + # Cases with zero counts match equivalents without counts (see gh-130285) + self.assertEqual( + sample('abc', k=0, counts=[0, 0, 0]), + sample([], k=0), + ) + self.assertEqual( + sample([], 0, counts=[]), + sample([], 0), + ) + with self.assertRaises(ValueError): + sample([], 1, counts=[]) + with self.assertRaises(ValueError): + sample('x', 1, counts=[0]) + def test_choices(self): choices = self.gen.choices data = ['red', 'green', 'blue', 'yellow'] From dfb81442b0d651c826d53d1fa58db7182ba07955 Mon Sep 17 00:00:00 2001 From: Raymond Hettinger Date: Tue, 18 Feb 2025 20:24:46 -0600 Subject: [PATCH 3/5] Fold try/except into a conditional expression --- Lib/random.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/Lib/random.py b/Lib/random.py index 2799b9b887d52f..beffa38a6bac92 100644 --- a/Lib/random.py +++ b/Lib/random.py @@ -421,10 +421,7 @@ def sample(self, population, k, *, counts=None): cum_counts = list(_accumulate(counts)) if len(cum_counts) != n: raise ValueError('The number of counts does not match the population') - try: - total = cum_counts.pop() - except IndexError: - total = 0 + total = cum_counts.pop() if cum_counts else 0 if not isinstance(total, int): raise TypeError('Counts must be integers') if total < 0: From 76749c527bb6f937164f52903d157b892bc1e7f3 Mon Sep 17 00:00:00 2001 From: Raymond Hettinger Date: Wed, 19 Feb 2025 18:28:38 -0600 Subject: [PATCH 4/5] Let the user focus on the source of the negative total --- Lib/random.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/random.py b/Lib/random.py index beffa38a6bac92..1abcae77c8be57 100644 --- a/Lib/random.py +++ b/Lib/random.py @@ -425,7 +425,7 @@ def sample(self, population, k, *, counts=None): if not isinstance(total, int): raise TypeError('Counts must be integers') if total < 0: - raise ValueError('Total of counts must be non-negative') + raise ValueError('Counts must be non-negative') selections = self.sample(range(total), k=k) bisect = _bisect return [population[bisect(cum_counts, s)] for s in selections] From 27f6cdb8cf2a2cf5b1c7818a33e641f53dd668bc Mon Sep 17 00:00:00 2001 From: Raymond Hettinger Date: Fri, 21 Feb 2025 10:32:51 -0600 Subject: [PATCH 5/5] Add blurb --- .../Library/2025-02-21-10-32-05.gh-issue-130285.C0fkh7.rst | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2025-02-21-10-32-05.gh-issue-130285.C0fkh7.rst diff --git a/Misc/NEWS.d/next/Library/2025-02-21-10-32-05.gh-issue-130285.C0fkh7.rst b/Misc/NEWS.d/next/Library/2025-02-21-10-32-05.gh-issue-130285.C0fkh7.rst new file mode 100644 index 00000000000000..7e0a4d219e385b --- /dev/null +++ b/Misc/NEWS.d/next/Library/2025-02-21-10-32-05.gh-issue-130285.C0fkh7.rst @@ -0,0 +1,4 @@ +Fix corner case for :func:`random.sample` allowing the *counts* parameter to +specify an empty population. So now, ``sample([], 0, counts=[])`` and +``sample('abc', k=0, counts=[0, 0, 0])`` both give the same result as +``sample([], 0)``.