diff --git a/CHANGELOG.md b/CHANGELOG.md index 7212e51..7582474 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,10 @@ Format: [Keep a Changelog](https://keepachangelog.com/en/1.1.0/) project's standing rules. ### Added +- Tests pinning the two untested branches of `WealthPercentileTransformer`: + the all-missing column path (returns NaN ranks, keeps a stable output + width, raises no warning) and the partially-missing column path (NaN + input rows get NaN rank, observed rows get numeric rank). Closes #169. - **`datasets.make_donor_panel`** (Tier 2, Beta): a seeded multi-year donor panel returning gift-level rows rather than one aggregated row per donor. `generate_synthetic_donor_data` cannot demonstrate `RFMTransformer` (needs a diff --git a/CONTRIBUTORS.md b/CONTRIBUTORS.md index 6ebc1aa..b40ba47 100644 --- a/CONTRIBUTORS.md +++ b/CONTRIBUTORS.md @@ -45,6 +45,10 @@ contribution. Code, docs, tests, and review all count. single-class fallback coverage for `PlannedGivingIntentScorer.predict_intent_score` ([#149](https://github.com/PhilanthroPy-Project/PhilanthroPy/pull/149)). +- **Lars** ([@Larslllllll](https://github.com/Larslllllll)): added the missing + unit-test coverage for `WealthPercentileTransformer`'s all-missing and + partially-missing column branches in `WealthPercentileTransformer` (closes + [#169](https://github.com/PhilanthroPy-Project/PhilanthroPy/issues/169)). ## Getting listed diff --git a/philanthropy/preprocessing/_wealth_percentile.py b/philanthropy/preprocessing/_wealth_percentile.py index 666a1df..4c5f178 100755 --- a/philanthropy/preprocessing/_wealth_percentile.py +++ b/philanthropy/preprocessing/_wealth_percentile.py @@ -43,9 +43,9 @@ def fit(self, X, y=None): """ X = validate_data(self, X, ensure_all_finite="allow-nan", reset=True) - if hasattr(X, "columns"): - self.feature_names_in_ = np.array(X.columns.tolist(), dtype=object) - elif not hasattr(self, "feature_names_in_"): + # validate_data has already set feature_names_in_ when input was a DataFrame, + # so we only need to handle the array-input path here. + if not hasattr(self, "feature_names_in_"): self.feature_names_in_ = np.array([f"x{i}" for i in range(X.shape[1])], dtype=object) # Use feature_names_in_ to resolve columns diff --git a/tests/test_preprocessing.py b/tests/test_preprocessing.py index 67acc77..ce10ba3 100755 --- a/tests/test_preprocessing.py +++ b/tests/test_preprocessing.py @@ -32,6 +32,7 @@ EncounterTransformer, FiscalYearTransformer, WealthScreeningImputer, + WealthPercentileTransformer, ) # Property-based tests for these transformers live in tests/test_properties.py. @@ -723,3 +724,93 @@ def test_crm_cleaner_on_unnamed_ndarrays_skips_named_columns(self): X = np.array([["2023-01-01", "1250.50"]], dtype=object) out = np.asarray(CRMCleaner().fit(X).transform(X)) assert out.shape == (1, 2) + + +# =========================================================================== +# 2. WealthPercentileTransformer: edge-case unit tests +# =========================================================================== + + +class TestWealthPercentileTransformer: + """Unit tests for WealthPercentileTransformer edge cases. + + These tests pin the behaviour of columns that are entirely missing or + partially missing, so the guard branches remain stable under future edits. + """ + + def test_all_missing_column_yields_nan_ranks(self): + # An all-NaN wealth column should store an empty reference array and + # return a column of NaN on transform -- NOT a RuntimeWarning or a + # column of inf. The output width is also preserved so downstream + # pipelines have a stable shape regardless of whether the wealth + # screen matched anything. + df = pd.DataFrame({ + "net_worth": [np.nan, np.nan, np.nan], + "other": [1.0, 2.0, 3.0], + }) + t = WealthPercentileTransformer().set_output(transform="pandas") + t.fit(df) + + # The guard branch stores an empty array + assert t.percentile_lookup_["net_worth"].size == 0 + + # Transform produces NaN in the rank column, other column unchanged + out = t.transform(df) + assert out["net_worth_pct_rank"].isna().all() + assert out["other"].tolist() == [1.0, 2.0, 3.0] + + # The rank column is still named and present (stable output width) + assert "net_worth_pct_rank" in out.columns + assert "net_worth_pct_rank" in list(t.get_feature_names_out()) + assert len(t.get_feature_names_out()) == 3 # net_worth, other, net_worth_pct_rank + + # No warning should be raised (would surface as an error here) + with warnings.catch_warnings(): + warnings.simplefilter("error") + t.transform(df) # must not warn + + def test_partially_missing_column_ranks_only_observed_values(self): + # When a wealth column has some NaN and some real values, only the + # real values contribute to the reference distribution; NaN rows get + # NaN in the rank column while non-NaN rows receive a numeric rank. + df = pd.DataFrame({ + "net_worth": [100_000.0, np.nan, 200_000.0, np.nan, 150_000.0], + "other": [1.0, 2.0, 3.0, 4.0, 5.0], + }) + t = WealthPercentileTransformer().set_output(transform="pandas") + out = t.fit_transform(df) + + assert "net_worth_pct_rank" in out.columns + # NaN input rows produce NaN rank + assert pd.isna(out.loc[1, "net_worth_pct_rank"]) + assert pd.isna(out.loc[3, "net_worth_pct_rank"]) + # Non-NaN input rows produce numeric rank + assert not pd.isna(out.loc[0, "net_worth_pct_rank"]) + assert not pd.isna(out.loc[2, "net_worth_pct_rank"]) + assert not pd.isna(out.loc[4, "net_worth_pct_rank"]) + # Other column passes through unchanged + assert out["other"].tolist() == [1.0, 2.0, 3.0, 4.0, 5.0] + + def test_wealth_percentile_feature_names_match_for_frame_and_array_input(self): + # Fit one transformer on a DataFrame and another on df.to_numpy(), + # then assert both produce the documented get_feature_names_out() shape: + # - DataFrame-fitted keeps real column names + # - array-fitted falls back to x0 .. xn + # This pins the behaviour the deleted hasattr(X, "columns") branch provided, + # so the deletion is provably safe rather than merely plausible. + df = pd.DataFrame({ + "net_worth": [1.0, 2.0, 3.0], + "other": [1.0, 2.0, 3.0], + }) + t_frame = WealthPercentileTransformer().set_output(transform="pandas") + t_array = WealthPercentileTransformer().set_output(transform="pandas") + + t_frame.fit(df) + t_array.fit(df.to_numpy()) + + # DataFrame-fitted transformer keeps real column names + assert list(t_frame.get_feature_names_out()) == ["net_worth", "other", "net_worth_pct_rank"] + + # Array-fitted transformer falls back to x0, x1 + assert list(t_array.get_feature_names_out()) == ["x0", "x1"] +