DEP: warn when drop_repeat_donors default is used (closes #108) - #147
Open
shubhrai23 wants to merge 1 commit into
Open
DEP: warn when drop_repeat_donors default is used (closes #108)#147shubhrai23 wants to merge 1 commit into
shubhrai23 wants to merge 1 commit into
Conversation
FiscalYearGroupedSplitter(drop_repeat_donors=...) now defaults to 'warn', which emits a DeprecationWarning telling the caller that the default will change from False to True in 0.8.0. Internally the splitter still treats 'warn' as False, so behaviour is unchanged. - Add deprecation entry to DEPRECATIONS registry in test_deprecations.py - Update all existing tests to pass drop_repeat_donors=False explicitly or wrap in pytest.warns(DeprecationWarning) - Silence doctests by passing drop_repeat_donors=False - Update CHANGELOG.md under [Unreleased] / Deprecated - Fix test_leakage.py to pass drop_repeat_donors=False explicitly Closes PhilanthroPy-Project#108
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
FiscalYearGroupedSplitter(drop_repeat_donors=...)currently defaults toFalse, which silently allows repeat donors across train and test folds. This PR deprecates that default so it will change toTruein 0.8.0, following the pattern established byWealthScreeningImputerKNN.group_col_idx.Changes
philanthropy/model_selection/_temporal_donor_splitter.pyFalseto the sentinel"warn".__init__now emits aDeprecationWarningwhen the default is left untouched.split()andget_n_splits()resolve"warn"toFalseinternally, so behaviour is unchanged... deprecated:: 0.7.0block.drop_repeat_donors=Falseexplicitly to avoid warnings.tests/test_model_selection.pytest_default_drop_repeat_donors_emits_deprecation_warning.test_explicit_drop_repeat_donors_false_silences_warning.drop_repeat_donors=Falseexplicitly or wrap inpytest.warns(DeprecationWarning).tests/test_deprecations.pyFiscalYearGroupedSplitter.drop_repeat_donorsto theDEPRECATIONSregistry so the meta-test passes.tests/test_leakage.pydrop_repeat_donors=Falseexplicitly.CHANGELOG.md[Unreleased] / Deprecated.Checklist
Closes #108