Skip to content

FiscalYearGroupedSplitter: drop_repeat_donors defaults to False, so the default path still leaks a static per-donor label across folds #108

Description

@shivamlalakiya

What's wrong

FiscalYearGroupedSplitter(drop_repeat_donors=...) defaults to False. For a
static per-donor label (e.g. is_major_donor, the label used in the README,
the benchmarks page, and scripts/benchmark_models.py), leaving it at the
default lets the same donor's rows land in both the train and test fold of a
split. That is the exact leakage this splitter exists to prevent, and it is
live in the default path, not an opt-in footgun.

Where

File Locate with
philanthropy/model_selection/_temporal_donor_splitter.py grep -n "drop_repeat_donors: bool = False" philanthropy/model_selection/_temporal_donor_splitter.py

What to change

  1. Add a DeprecationWarning in FiscalYearGroupedSplitter.split (or
    __init__) when drop_repeat_donors is left at its default False,
    stating that the default will become True in a future release and that
    callers relying on repeat donors appearing in both folds should pass
    drop_repeat_donors=False explicitly to silence it.
  2. Update the class docstring's drop_repeat_donors parameter entry to state
    the planned default flip and the release it lands in, following the same
    pattern already used for WealthScreeningImputerKNN(group_col_idx=...)
    (see CHANGELOG.md, ## [Unreleased] section, Deprecated).
  3. Add a ## [Unreleased] CHANGELOG.md entry and a CONTRIBUTORS.md line
    in the same PR (required by AGENTS.md).

Tests to add or extend

  • File: tests/test_temporal_donor_splitter.py
  • Add: test_default_drop_repeat_donors_emits_deprecation_warning
  • Add: test_explicit_drop_repeat_donors_false_silences_warning

Done when

python -c "
import warnings
from philanthropy.model_selection import FiscalYearGroupedSplitter
with warnings.catch_warnings(record=True) as w:
    warnings.simplefilter('always')
    FiscalYearGroupedSplitter()
    assert any(issubclass(x.category, DeprecationWarning) for x in w)
"
make ci

First time here?

Read CONTRIBUTING.md and AGENTS.md.
Add a ## [Unreleased] CHANGELOG entry and yourself to CONTRIBUTORS.md in
the same PR.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions