Migrate WoEEncoder to narwhals, add polars support - #1032
Merged
Merged
Conversation
solegalli
force-pushed
the
narwhals-woe-encoder
branch
2 times, most recently
from
August 30, 2026 22:52
6505e37 to
2397f9a
Compare
solegalli
force-pushed
the
narwhals-woe-encoder
branch
from
September 14, 2026 20:46
2397f9a to
b19aab3
Compare
Collaborator
Author
|
Updated this branch:
Locally: |
solegalli
force-pushed
the
narwhals-woe-encoder
branch
from
September 15, 2026 10:01
b19aab3 to
0c0612e
Compare
solegalli
force-pushed
the
narwhals-woe-encoder
branch
from
September 15, 2026 11:48
0c0612e to
69bc34f
Compare
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>
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>
Replace the file-local data dicts and assert_df_equal/_none_to_nan helpers with the shared test structure: make_df and data_enc* fixtures, y built with make_series on the backend under test, isinstance(X, make_df) plus to_dict() checks, and pytest.raises/warns(match=re.escape(msg)). Add a test passing the target as a list and as a numpy array, which take a different code path than a Series. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
solegalli
force-pushed
the
narwhals-woe-encoder
branch
from
September 15, 2026 12:00
69bc34f to
4acf874
Compare
… SelectByInformationValue Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ith_zero_counts_ Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Depends on #1047 (shared encoder helper and init checks).
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.