PhilanthroPy-Project · shivamlalakiya · Sep 24, 2026 · Sep 24, 2026
@@ -73,6 +73,14 @@ Format: [Keep a Changelog](https://keepachangelog.com/en/1.1.0/)
   mkdocstrings-generated API reference.
 
 ### Changed
+- **Breaking:** `FiscalYearGroupedSplitter(drop_repeat_donors=...)` now
+  defaults to `True`, as the `DeprecationWarning` in 0.7.0 and 0.7.1 said it
+  would. Each test fold drops donors already seen in its training rows, which
+  is the safe default for a static per-donor label. It needs `groups` as
+  `(n_samples, 2)` (fiscal year, donor id); code that passes fiscal years alone
+  now raises a `ValueError` that names both fixes. Pass
+  `drop_repeat_donors=False` for a time-varying target to keep the 0.7.x
+  behaviour. The two leakage experiment scripts now pass it explicitly.
 - The docs homepage hero no longer uses an all-caps eyebrow label or a
   gradient-clipped headline; it's now a two-column layout with the headline
   beside a real ranked-donor ledger table showing what
@@ -121,6 +129,12 @@ Format: [Keep a Changelog](https://keepachangelog.com/en/1.1.0/)
   Closes #195.
 
 ### Removed
+- **Breaking:** `philanthropy.utils.make_donor_dataset`, deprecated since
+  0.7.0. Import it from `philanthropy.datasets`.
+- **Breaking:** `WealthScreeningImputerKNN(group_col_idx=...)` and its
+  `group_imputers_` attribute, deprecated since 0.7.0. Per-group and global KNN
+  fits were measured bit-identical, so the parameter bought nothing; passing it
+  is now a `TypeError`. `tests/test_knn_group_stratification.py` goes with it.
 - Deleted `philanthropy/preprocessing/_solicitation_window.py`, a dead module
   nothing imported. The deprecated `SolicitationWindowTransformer` alias it held
   was already served by the subpackage's PEP 562 module-level `__getattr__`, so

@@ -127,11 +127,16 @@ freeze them: `metrics.scoring` → `metrics._scoring`,
 subpackage (`from philanthropy.metrics import ...`), as the documented examples
 always have, and nothing breaks.
 
-### Live in 0.7.0, removed in 0.8.0
+### Removed or changed in 0.8.0
 
-| Deprecated | Use instead |
+If you are upgrading from 0.7.x, these went after two published releases of
+`DeprecationWarning`:
+
+| Removed or changed | Use instead |
 |---|---|
+| `philanthropy.utils.make_donor_dataset` | `philanthropy.datasets.make_donor_dataset` |
 | `WealthScreeningImputerKNN(group_col_idx=...)` | nothing; see below |
+| `FiscalYearGroupedSplitter(drop_repeat_donors=...)` default is now `True` | pass donor ids, or `drop_repeat_donors=False` for a time-varying target |
 
 `group_col_idx` has no replacement because there is nothing to replace. It was
 documented for a long time as stratifying KNN imputation per group "improving
@@ -141,11 +146,20 @@ pools and on five Python versions in CI, per-group and global KNN imputation
 produce **bit-identical** output (`50263.48615163204` both ways). A donor's
 nearest neighbours by feature distance almost always share their group already,
 and `KNNImputer` weights distance by column magnitude, so a 0/1 group flag
-barely registers.
+barely registers. Passing it is now a `TypeError`.
 
 If you need per-group behaviour, split the frame by group and fit one imputer per
 part. That is explicit, and it costs nothing that the parameter was buying.
 
+`FiscalYearGroupedSplitter` now drops, from each test fold, donors already seen
+in that fold's training rows. That is the safe choice for a static per-donor
+label such as `is_major_donor`, which a model can otherwise memorise from the
+donor's earlier years. It needs `groups` as `(n_samples, 2)`: fiscal year, then
+donor id. Code that passes fiscal years alone now raises a `ValueError` naming
+both fixes. For a time-varying target such as "gave next year", where the same
+donor correctly appears in both folds, pass `drop_repeat_donors=False`, which
+gives exactly the 0.7.x behaviour.
+
 ### Live in 0.7.1, removed in 0.9.0
 
 | Deprecated | Use instead |

@@ -72,26 +72,26 @@ class FiscalYearGroupedSplitter(BaseCrossValidator):
         the fiscal year immediately before the test year is withheld from
         training (useful when gift officers use current-year pipeline
         intelligence that would not have been available historically).
-    drop_repeat_donors : bool, default=False
-        .. deprecated:: 0.7.0
-            Leaving ``drop_repeat_donors`` at its default emits a
-            ``DeprecationWarning``. The default changes to ``True`` in 0.8.0.
-            Pass ``drop_repeat_donors=False`` explicitly to silence this warning
-            and keep the current behaviour.
+    drop_repeat_donors : bool, default=True
+        .. versionchanged:: 0.8.0
+            The default changed from ``False`` to ``True``, after 0.7.0 and
+            0.7.1 warned about it. Code that relied on the old default and
+            passes one-dimensional ``groups`` now raises a ``ValueError``
+            asking for either donor ids or ``drop_repeat_donors=False``.
 
         Whether to remove from each test fold any donor who already appears in
         that fold's training rows.
 
-        Leave this ``False`` for a **time-varying** target such as "did this
-        donor give in FY22?". There, a donor appearing in both folds is correct:
-        the training rows precede the test rows in time, which is the point of
-        walk-forward evaluation.
-
-        Set it ``True`` for a **static per-donor** label such as
+        Keep it ``True`` for a **static per-donor** label such as
         ``is_major_donor``, where the same answer is attached to every one of
         that donor's rows, so the model can memorise it from the donor's earlier
         years. That is the leakage described under "What this does not prevent"
-        below.
+        below, and the reason this is the default.
+
+        Set it ``False`` for a **time-varying** target such as "did this donor
+        give in FY22?". There, a donor appearing in both folds is correct: the
+        training rows precede the test rows in time, which is the point of
+        walk-forward evaluation.
 
         When ``True``, ``groups`` must be two-dimensional with shape
         ``(n_samples, 2)``: column 0 the fiscal year, column 1 the donor
@@ -166,8 +166,8 @@ class FiscalYearGroupedSplitter(BaseCrossValidator):
     the same answer is attached to every one of that donor's rows and the model
     can memorise it from the training years.
 
-    For that case, set ``drop_repeat_donors=True`` and pass ``groups`` as
-    ``(n_samples, 2)`` with the donor identifier in column 1. Each test fold then
+    For that case, keep the default ``drop_repeat_donors=True`` and pass
+    ``groups`` as ``(n_samples, 2)`` with the donor identifier in column 1. Each test fold then
     excludes donors already present in its training rows. Aggregating to one row
     per donor and using a grouped holdout remains the cleaner option when the
     label has no time dimension at all.
@@ -185,22 +185,12 @@ def __init__(
         self,
         n_splits: int = 5,
         gap_years: int = 0,
-        drop_repeat_donors: bool | str = "warn",
+        drop_repeat_donors: bool = True,
     ) -> None:
         # MUST call super().__init__() for BaseCrossValidator compat.
         self.n_splits = n_splits
         self.gap_years = gap_years
         self.drop_repeat_donors = drop_repeat_donors
-
-        if self.drop_repeat_donors == "warn":
-            warnings.warn(
-                "The FiscalYearGroupedSplitter(drop_repeat_donors=...) default "
-                "of False is deprecated and allows repeat donors across train "
-                "and test folds. This default will change to True in 0.8.0. Pass "
-                "drop_repeat_donors=False explicitly to silence this warning.",
-                DeprecationWarning,
-                stacklevel=2,
-            )
 
     # ------------------------------------------------------------------
     # Required abstract-method implementations
@@ -270,7 +260,7 @@ def split(
             If ``drop_repeat_donors=True`` empties a test fold entirely.
         """
         requested_splits, gap_years = self._validate_params()
-        drop_repeat = False if self.drop_repeat_donors == "warn" else bool(self.drop_repeat_donors)
+        drop_repeat = bool(self.drop_repeat_donors)
 
         if groups is None:
             raise ValueError(
@@ -284,9 +274,12 @@ def split(
         if drop_repeat:
             if groups_arr.ndim != 2 or groups_arr.shape[1] != 2:
                 raise ValueError(
-                    "drop_repeat_donors=True requires `groups` with shape "
-                    "(n_samples, 2): column 0 the fiscal year, column 1 the "
-                    f"donor identifier. Got shape {groups_arr.shape}."
+                    "drop_repeat_donors=True (the default since 0.8.0) requires "
+                    "`groups` with shape (n_samples, 2): column 0 the fiscal "
+                    "year, column 1 the donor identifier. Got shape "
+                    f"{groups_arr.shape}. For a time-varying target such as "
+                    "'gave next year', where a donor belongs in both folds, "
+                    "pass drop_repeat_donors=False with fiscal years alone."
                 )
             donor_ids = groups_arr[:, 1]
             groups_arr = groups_arr[:, 0]
@@ -407,7 +400,7 @@ def get_n_splits(
         n_splits, gap_years = self._validate_params()
         if groups is not None:
             groups = np.asarray(groups)
-            drop_repeat = False if self.drop_repeat_donors == "warn" else bool(self.drop_repeat_donors)
+            drop_repeat = bool(self.drop_repeat_donors)
             if drop_repeat and groups.ndim == 2 and groups.shape[1] == 2:
                 groups = groups[:, 0]
             unique_fy = np.unique(groups)

@@ -87,56 +87,13 @@ class WealthScreeningImputerKNN(TransformerMixin, BaseEstimator):
         Append a binary ``<col>__was_missing`` column for each imputed
         wealth column.  Strongly recommended: absence of vendor records
         itself carries predictive signal.
-    group_col_idx : int or None, default=None
-        .. deprecated:: 0.7.0
-            Passing ``group_col_idx`` emits a ``DeprecationWarning`` and the
-            parameter is removed in 0.8.0. It still works meanwhile; there is no
-            replacement, because there is nothing to replace.
-
-            The reason is measurement, not tidiness. Across several synthetic
-            two-group pools, and on five Python versions in CI, the grouped and
-            global fits produce **bit-identical** output: 50263.48615163204 both
-            ways. That is not a near miss. A donor's nearest neighbours by
-            feature distance almost always share their group already, so
-            restricting the fit to the group changes nothing, and
-            :class:`~sklearn.impute.KNNImputer` weights distance by column
-            magnitude, so a 0/1 group flag contributes almost nothing on its own.
-            The parameter costs a per-group imputer, three fallback paths and a
-            documented contract, and buys no measurable accuracy. If you need
-            per-group behaviour, split the frame by group and fit one imputer per
-            part, which is explicit and costs nothing here.
-
-        Column index of a group variable (for example a zip code encoded as an
-        int) to stratify KNN imputation. When set and ``strategy="knn"``, a
-        separate :class:`~sklearn.impute.KNNImputer` is fitted per group, so a
-        donor's missing wealth is filled from neighbours inside their own group
-        rather than from the whole database. Ignored for the other strategies,
-        which are columnwise statistics with no notion of a neighbourhood.
-
-        Two fallbacks, both frozen at :meth:`fit` time so nothing is learned at
-        transform time:
-
-        * A group with fewer than ``n_neighbors + 1`` training rows gets no
-          imputer of its own, because KNN over too few neighbours is worse than
-          the global fit. Its rows use the global imputer.
-        * A group value **not seen during fit**, or a row whose group value is
-          missing, also uses the global imputer. This is the leakage-safe
-          choice: the alternative is fitting on the data being transformed.
-
-        The global imputer is always fitted, so the output is never ``NaN``
-        regardless of grouping.
 
     Attributes
     ----------
     imputed_cols_ : list of str
         Wealth columns that were actually present in ``X`` at fit time.
     fill_values_ : dict of {str: float}
         Fill statistics (only populated for non-KNN strategies).
-    group_imputers_ : dict of {float: tuple}
-        Maps each group value that qualified for its own imputer to
-        ``(fitted KNNImputer, boolean mask of columns entirely missing within
-        that group)``. Empty when ``group_col_idx`` is ``None`` or ``strategy``
-        is not ``"knn"``.
     knn_imputer_ : KNNImputer or None
         The fitted :class:`~sklearn.impute.KNNImputer` instance
         (only populated for ``strategy="knn"``).
@@ -169,13 +126,11 @@ def __init__(
         strategy: Literal["median", "mean", "zero", "knn"] = "knn",
         n_neighbors: int = 5,
         add_indicator: bool = True,
-        group_col_idx: Optional[int] = None,
     ) -> None:
         self.wealth_cols = wealth_cols
         self.strategy = strategy
         self.n_neighbors = n_neighbors
         self.add_indicator = add_indicator
-        self.group_col_idx = group_col_idx
 
     def _resolve_cols(self, input_cols: list[str]) -> list[str]:
         if self.wealth_cols is not None:
@@ -195,19 +150,6 @@ def fit(self: _SelfK, X: Any, y: Any = None) -> _SelfK:
         -------
         self : WealthScreeningImputerKNN
         """
-        if self.group_col_idx is not None:
-            warnings.warn(
-                "WealthScreeningImputerKNN(group_col_idx=...) is deprecated "
-                "since 0.7.0 and will be removed in 0.8.0. It has no "
-                "replacement: measured across several synthetic pools and five "
-                "Python versions in CI, per-group and global KNN imputation "
-                "produce bit-identical output, so the parameter buys no "
-                "accuracy. Split the frame by group and fit one imputer per part "
-                "if you need that behaviour.",
-                DeprecationWarning,
-                stacklevel=2,
-            )
-
         if self.strategy not in self._VALID_STRATEGIES:
             raise ValueError(
                 f"`strategy` must be one of {sorted(self._VALID_STRATEGIES)}, "
@@ -248,51 +190,15 @@ def fit(self: _SelfK, X: Any, y: Any = None) -> _SelfK:
 
         if self.strategy == "knn":
             # Fit KNNImputer on ALL columns (preserves inter-column structure).
-            # Always fitted, even when grouping: it is the fallback for small and
-            # unseen groups, and it is what guarantees the output has no NaN.
             self.knn_imputer_: Optional[KNNImputer] = KNNImputer(
                 n_neighbors=self.n_neighbors,
                 weights="distance",
                 keep_empty_features=True,
             )
             self.knn_imputer_.fit(X_arr)
             self.fill_values_: dict[str, float] = {}
-            self.group_imputers_: dict = {}
-            if self.group_col_idx is not None:
-                gidx = int(self.group_col_idx)
-                if not -X_arr.shape[1] <= gidx < X_arr.shape[1]:
-                    raise ValueError(
-                        f"`group_col_idx` {gidx} is out of range for X with "
-                        f"{X_arr.shape[1]} columns."
-                    )
-                groups = X_arr[:, gidx]
-                # A group needs more rows than neighbours for KNN to mean
-                # anything; smaller groups deliberately get no imputer and fall
-                # back to the global one.
-                min_rows = self.n_neighbors + 1
-                for value in np.unique(groups[~np.isnan(groups)]):
-                    rows = groups == value
-                    if int(rows.sum()) < min_rows:
-                        continue
-                    sub_imputer = KNNImputer(
-                        n_neighbors=self.n_neighbors,
-                        weights="distance",
-                        keep_empty_features=True,
-                    )
-                    sub_imputer.fit(X_arr[rows])
-                    # Columns entirely missing WITHIN this group. KNNImputer with
-                    # keep_empty_features=True fills those with a hard 0.0, not
-                    # NaN, so a NaN check at transform time cannot see them. For a
-                    # wealth column, 0.0 reads as "no capacity", which is a
-                    # materially wrong answer rather than a missing one. Record
-                    # them so transform defers to the global imputer instead.
-                    self.group_imputers_[float(value)] = (
-                        sub_imputer,
-                        np.isnan(X_arr[rows]).all(axis=0),
-                    )
         else:
             self.knn_imputer_ = None
-            self.group_imputers_ = {}
             fills: dict[str, float] = {}
             for col in self.imputed_cols_:
                 idx = col_indices[col]
@@ -353,23 +259,7 @@ def transform(self, X: Any, y: Any = None) -> np.ndarray:
                     indicators.append(np.isnan(X_arr[:, idx]).astype(np.float64).reshape(-1, 1))
 
         if self.strategy == "knn" and self.knn_imputer_ is not None:
-            # Global result first: this is the fallback for small groups, unseen
-            # groups and missing group labels, and it guarantees no NaN survives.
             X_out = self.knn_imputer_.transform(X_arr)
-            group_imputers = getattr(self, "group_imputers_", {})
-            if group_imputers and self.group_col_idx is not None:
-                groups = X_arr[:, int(self.group_col_idx)]
-                for value, (sub_imputer, empty_cols) in group_imputers.items():
-                    rows = groups == value
-                    if not rows.any():
-                        continue
-                    X_group = sub_imputer.transform(X_arr[rows])
-                    # Prefer the group-local value, except where it cannot be
-                    # trusted: a NaN it failed to fill, or a column entirely
-                    # missing inside this group, where KNNImputer returns a hard
-                    # 0.0 that would read as real data.
-                    reject = np.isnan(X_group) | empty_cols[None, :]
-                    X_out[rows] = np.where(reject, X_out[rows], X_group)
         else:
             X_out = X_arr.copy()
             for col in self.imputed_cols_:

@@ -1,29 +1,9 @@
 """
 philanthropy.utils
 ==================
-Generic helpers: model persistence and deprecated aliases.
+Generic helpers: model persistence.
 """
 
-import warnings
-from typing import Any
-
-import pandas as pd
-
 from ._persistence import save_model, load_model
 
-
-def make_donor_dataset(*args: Any, **kwargs: Any) -> pd.DataFrame:
-    """Deprecated alias for :func:`philanthropy.datasets.make_donor_dataset`."""
-    warnings.warn(
-        "philanthropy.utils.make_donor_dataset is deprecated and will be "
-        "removed in 0.8.0; import make_donor_dataset from philanthropy.datasets "
-        "instead.",
-        DeprecationWarning,
-        stacklevel=2,
-    )
-    from ..datasets import make_donor_dataset as _make_donor_dataset
-
-    return _make_donor_dataset(*args, **kwargs)
-
-
-__all__ = ["make_donor_dataset", "save_model", "load_model"]
+__all__ = ["save_model", "load_model"]
@@ -116,8 +116,8 @@ def main():
         as_of, whole = _panel(seed)
         truth.append(_true_future(as_of))
         random_cv.append(_cv(as_of, StratifiedKFold(5, shuffle=True, random_state=0), False))
-        walk_cv.append(_cv(as_of, FiscalYearGroupedSplitter(n_splits=3), True))
-        whole_cv.append(_cv(whole, FiscalYearGroupedSplitter(n_splits=3), True))
+        walk_cv.append(_cv(as_of, FiscalYearGroupedSplitter(n_splits=3, drop_repeat_donors=False), True))
+        whole_cv.append(_cv(whole, FiscalYearGroupedSplitter(n_splits=3, drop_repeat_donors=False), True))
 
     print(f"Donor-year panel: {N_DONORS} donors x {len(YEARS) - 1} panel years, "
           f"label = gave in the following year.")