BUG: Require sample weights to sum to less than 1 when replace = True #61582

microslaw · 2025-06-06T10:05:48Z

closes BUG: DataFrame.sample weights not required to sum to less than 1 #61516
Tests added and passed if fixing a bug or adding a new feature
All code checks passed.
Added an entry in the latest doc/source/whatsnew/vX.X.X.rst file if fixing a bug or adding a new feature.

rhshadrach

Looking good, but the condition needs adjusted. Will also need a note in the whatsnew for 3.0 under the Other section.

rhshadrach · 2025-06-06T18:45:38Z

pandas/core/sample.py

+        if weight_sum > 1 and replace:
+            raise ValueError(
+                "Invalid weights: If `replace`=True weights must sum to less than 1"
+            )


This is not the correct condition. See the formula in the OP of the linked issue.

yuanx749 · 2025-06-06T21:07:58Z

pandas/core/generic.py

+            When replace = True will not allow weights that add up to less
+            than 1, to avoid biased results.


Suggested change

When replace = True will not allow weights that add up to less

than 1, to avoid biased results.

When replace = False will not allow ``(n * max(weights) / sum(weights)) > 1``,

to avoid biased results.

microslaw · 2025-06-13T21:08:28Z

I've added this as an exception, but I've also considered doing this as a warning, since the issue is
- easy to encounter - one case occured in user_guide
- hard to explain within a short error message - requires specific equation and some level of understanding of probability
- has minimal effect on majority of use cases - as long as the variance isn't huge, the bias this error introduces is minimal, and for very specific cases e.g. the village sampling from the op warning should suffice
I also deviated a bit from original issue in that i'm allowing max(w) * n / sum (w) == 1. As far as I understand, it doesn't introduce any bias, and it allows some specific uses, e.g. df.sample(n=1,weights=[1,0])
If this is implemented as an exception, I believe that there is no way for the numpy exception "Fewer non-zero entries in p than size" to occur, as:
- w - list of weights
- n - number of non-zero weights in w
- size - size of the sample to be drawn
- len - length of w
I'm assuming that
- the weights are normalized, so sum(w) = 1, as it doesn't change anything and makes the math easier
- the weights are equal, since if any of them is larger than the others, max(w) and max(w) * size will increase
- therefore w = [1/n, 1/n, ..., 1/n] or any combination like w = [1/n, 0, 1/n, 0, ..., 1/n] where n is the number of non-zero weights
- for the "Fewer non-zero entries in p than size" to occur, size > n
- for the "Invalid weights: If replace=False, " to occur, size * max(w) / sum(w) >= 1
- size * max(w) / sum(w) >= 1
- size * max(w) >= 1, as sum(w) = 1
- size * 1/n >= 1, as max(w) = 1/n
- size >= n

rhshadrach

Looking good!

rhshadrach · 2025-06-21T14:11:01Z

pandas/core/sample.py

+    if weights is not None:
+        is_max_weight_dominating = size * weights.max() > 1
+        if is_max_weight_dominating and not replace:
+            raise ValueError(
+                "Invalid weights: If `replace`=False, "
+                "total unit probabilities have to be less than 1"
+            )


Avoid O(n) computation if it is not needed.

Combine with the if block above; no need to check if weights is not None twice.

Suggestion on a more clear error message.

Suggested change

if weights is not None:

is_max_weight_dominating = size * weights.max() > 1

if is_max_weight_dominating and not replace:

raise ValueError(

"Invalid weights: If `replace`=False, "

"total unit probabilities have to be less than 1"

)

if not replace and size * weights.max() > 1:

raise ValueError(

"Weighted sampling cannot be achieved with replace=False. Either "

"set replace=True or use smaller weights. See the docstring of sample "

"for details."

)

rhshadrach · 2025-06-21T14:12:34Z

pandas/core/generic.py

            Missing values in the weights column will be treated as zero.
            Infinite values not allowed.
+            When replace = False will not allow ``(n * max(weights) / sum(weights)) > 1``,
+            in order to avoid biased results.


Can you add "See the Notes below for more details." and then add a bit more detail in the notes section. I can give it a shot if you'd prefer.

rhshadrach · 2025-06-21T14:13:29Z

pandas/tests/frame/methods/test_sample.py

+        # edge case, n*max(weights)/sum(weights) == 1
+        edge_variance_weights = [1] * 10
+        edge_variance_weights[0] = 9
+        # should not raise
+        obj.sample(n=2, weights=edge_variance_weights, replace=False)
+
+        low_variance_weights = [1] * 10
+        low_variance_weights[0] = 8
+        # should not raise
+        obj.sample(n=2, weights=low_variance_weights, replace=False)


Can you make these two separate tests.

rhshadrach

Thanks for the chnages. A few minor requests and we should be good to merge.

rhshadrach · 2025-07-01T10:42:17Z

doc/source/whatsnew/v0.16.1.rst

-   df = pd.DataFrame({"col1": [9, 8, 7, 6], "weight_column": [0.5, 0.4, 0.1, 0]})
-   df.sample(n=3, weights="weight_column")
+      df = pd.DataFrame({"col1": [9, 8, 7, 6], "weight_column": [0.5, 0.4, 0.1, 0]})
+      df.sample(n=2, weights="weight_column")


The added indentation here looks incorrect.

rhshadrach · 2025-07-01T10:43:24Z

pandas/core/generic.py

            If weights do not sum to 1, they will be normalized to sum to 1.
            Missing values in the weights column will be treated as zero.
            Infinite values not allowed.
+            When replace = False will not allow ``(n * max(weights) / sum(weights)) > 1``,


nit: remove the , at the end of this line

rhshadrach · 2025-07-01T10:44:54Z

pandas/core/generic.py

        -----
        If `frac` > 1, `replacement` should be set to `True`.

+        When replace = False will not allow ``(n * max(weights) / sum(weights)) > 1``,


nit: remove the trailing ,.

rhshadrach · 2025-07-01T10:45:10Z

pandas/core/generic.py


+        When replace = False will not allow ``(n * max(weights) / sum(weights)) > 1``,
+        since that would cause results to be biased. E.g. sampling 2 items without replacement,
+        with weights [100, 1, 1] would yield two last items in 1/2 of cases, instead of 1/102


Suggested change

with weights [100, 1, 1] would yield two last items in 1/2 of cases, instead of 1/102

with weights ``[100, 1, 1]`` would yield two last items in 1/2 of cases, instead of 1/102.

Can you also add, "This is similar to specifying n=4 without replacement on a Series with 3 elements."

rhshadrach

lgtm

rhshadrach · 2025-07-11T02:27:23Z

Thanks @microslaw!

…pandas-dev#61582)

microslaw force-pushed the add-df-sample-weight-constraints branch 2 times, most recently from 1281002 to 1d1f742 Compare June 6, 2025 11:15

mroeschke requested a review from rhshadrach June 6, 2025 16:38

rhshadrach requested changes Jun 6, 2025

View reviewed changes

yuanx749 reviewed Jun 6, 2025

View reviewed changes

microslaw force-pushed the add-df-sample-weight-constraints branch 3 times, most recently from 2bc3618 to e5f9a07 Compare June 6, 2025 21:53

microslaw marked this pull request as draft June 6, 2025 21:58

microslaw force-pushed the add-df-sample-weight-constraints branch 5 times, most recently from 9f62a65 to 11df613 Compare June 12, 2025 22:32

microslaw marked this pull request as ready for review June 13, 2025 21:07

microslaw requested a review from rhshadrach June 13, 2025 21:08

rhshadrach requested changes Jun 21, 2025

View reviewed changes

simonjayhawkins added Bug Algos Non-arithmetic algos: value_counts, factorize, sorting, isin, clip, shift, diff labels Jun 25, 2025

microslaw force-pushed the add-df-sample-weight-constraints branch 4 times, most recently from 5f7484b to 7e7a73c Compare June 27, 2025 15:07

rhshadrach requested changes Jul 1, 2025

View reviewed changes

microslaw added 5 commits July 8, 2025 18:46

implement the exception

46e70ef

remove impossible test scenario

d18c74b

add new tests

f562384

update documentation

e4e2cbf

tmp

c4241fb

microslaw force-pushed the add-df-sample-weight-constraints branch from f553ed7 to c4241fb Compare July 8, 2025 18:46

doc: minior adjustments

7b9aac4

microslaw requested a review from rhshadrach July 9, 2025 04:59

rhshadrach approved these changes Jul 11, 2025

View reviewed changes

rhshadrach modified the milestones: 2.3.2, 3.0 Jul 11, 2025

rhshadrach merged commit 9da2c8f into pandas-dev:main Jul 11, 2025
44 checks passed

eicchen pushed a commit to eicchen/pandas that referenced this pull request Aug 19, 2025

BUG: Require sample weights to sum to less than 1 when replace = True (…

0958dfd

…pandas-dev#61582)

		When replace = True will not allow weights that add up to less
		than 1, to avoid biased results.

	with weights [100, 1, 1] would yield two last items in 1/2 of cases, instead of 1/102
	with weights ``[100, 1, 1]`` would yield two last items in 1/2 of cases, instead of 1/102.

Uh oh!

BUG: Require sample weights to sum to less than 1 when replace = True #61582

BUG: Require sample weights to sum to less than 1 when replace = True #61582

Uh oh!

Conversation

microslaw commented Jun 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rhshadrach left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

microslaw commented Jun 13, 2025

Uh oh!

rhshadrach left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rhshadrach left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rhshadrach left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rhshadrach commented Jul 11, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

microslaw commented Jun 6, 2025 •

edited

Loading