Migrate WoEEncoder to narwhals, add polars support - #1032
Open
solegalli wants to merge 2 commits into
Open
Conversation
fit() splits by backend: pandas keeps _calculate_woe()'s existing two-groupby implementation unchanged (it's directly unit-tested for that exact pandas-Series-with-category-index contract); polars/other narwhals backends use one group_by() instead of two, deriving the negative-class count as the complement of the positive-class count per category - benchmarked competitive with, and often faster than, pandas-native at 50k-100k rows. Zero-count-per-class fill_value handling preserved exactly. Bug fix: _check_fit_input() previously assumed y was always a pandas Series (y.nunique()/y.min()/y.max()), breaking on a numpy y (e.g. a plain list/array-like target, which sklearn's check_X_y machinery converts via column_or_1d). Wrapped numpy y into a narwhals Series aligned to X's backend; for pandas specifically, also had to line the wrapped Series up with X's actual index, since _calculate_woe()'s y.groupby(X[var]) aligns by index and a mismatched default RangeIndex silently drops every row instead of raising, leaving encoder_dict_ empty. Fixes test_encoders_when_x_pandas_y_numpy's WoEEncoder case (was failing on the unmigrated file, confirmed pre-existing). Verified: 44/44 own tests, full encoding suite 342 passed/16 failed (was 17 pre-existing on the narwhals-encoding-base baseline - one less here since this branch's own numpy-y bug is now fixed, rest confirmed unrelated), flake8 and mypy clean, sphinx -W build clean (only the pre-existing unrelated linkcode_resolve warning). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
solegalli
force-pushed
the
narwhals-woe-encoder
branch
from
August 30, 2026 22:48
b1b7c2f to
6505e37
Compare
check_X_y now returns a narwhals frame. In _check_fit_input, bind that to nw_X and keep the original native X: the nwd.is_pandas_dataframe(X) check, the native_y.index = X.index alignment and the returned X all need native input, and fit()'s pandas _calculate_woe fast path and nwd checks are then unchanged (X stays native so no rehydration is needed). Take the y-series backend from nw_X.implementation instead of re-wrapping X. In transform(), bind _check_transform_input_and_state to nw_X, keep native X for _check_contains_na, and pass nw_X to _encode (which now expects narwhals). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
solegalli
force-pushed
the
narwhals-woe-encoder
branch
from
August 30, 2026 22:52
6505e37 to
2397f9a
Compare
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
1 participant
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.
Migrates
WoEEncoderto narwhals with polars support.fit()splits by backend: pandas keeps_calculate_woe()'s existing two-groupbyimplementation unchanged (it's directly unit-tested for that exact pandas-Series-with-category-index contract); polars/other narwhals backends use onegroup_by(), deriving the negative-class count as the complement of the positive-class count per category — benchmarked competitive with, and often faster than, pandas-native at 50k–100k rows. Zero-count-per-classfill_valuehandling preserved exactly.Bug fix:
_check_fit_input()assumedywas always a pandas Series (y.nunique()/y.min()/y.max()), breaking on a numpyy(list/array-like target, which sklearn'scheck_X_yconverts viacolumn_or_1d). Wrapped numpyyinto a narwhals Series aligned toX's backend; for pandas, also aligned it toX's actual index, since_calculate_woe()'sy.groupby(X[var])aligns by index and a mismatched defaultRangeIndexsilently drops every row instead of raising, leavingencoder_dict_empty. Fixestest_encoders_when_x_pandas_y_numpy'sWoEEncodercase (was failing on the unmigrated file — confirmed pre-existing).Verified: 44/44 own tests; full encoding suite 342 passed / 16 failed (was 17 pre-existing on the
narwhals-encoding-basebaseline — one fewer here since this branch's own numpy-ybug is now fixed, rest confirmed unrelated). flake8 / mypy clean, sphinx -W clean.Stacked on #999 (
narwhals-encoding-base). Until that merges this PR's diff also contains the sharedCategoricalMethodsMixincommit; review #999 first.