Conversation
…s support BaseSelector.transform() returns the retained features in the train set order, in the same library as the input (pandas X[features], narwhals select otherwise). BaseRecursiveSelector.fit() trains the estimators on native frames and returns (nw_X, y). The helpers in base_selection_functions no longer import pandas: correlations are computed with numpy (np.corrcoef, or matrix products for pairwise complete observations when there are missing values), and feature importances are pandas Series for pandas input and dicts otherwise. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
- Compute the information values with numpy on pandas and with one lazy narwhals query on other backends. - Discretise numerical variables into integer codes instead of interval labels, and skip the datetime parsing when finding the numerical variables. - Validate confirm_variables at init, like the other selectors. - Rewrite the tests to run on pandas and polars, and update the docstring and user guide. 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.
Stacked on #1070 (selection base classes). Its commit shows in this diff until #1070 is merged.
Summary
SelectByInformationValuenow works with pandas and polars, and returns the same library it receives (transformis inherited fromBaseSelector).fit: the information values are computed with numpy on pandas (Series.factorize()+np.bincount), and with one lazy narwhals query over all the variables on other backends (polars runs the per-variable group-bys in parallel).return_boundaries=False) instead of interval labels: the IV only needs to know which interval each value falls in, and grouping integers is faster than building and grouping strings.find_categorical_and_numerical_variables(..., exclude_datetime=False):variables_has already been filtered for datetime by_select_all_variables, and the datetime check (which parses the values) doesn't change the numerical list. This check was the largest single cost offit.WoEEncodernow uses onnarwhals-migration. The selector no longer callsWoE._calculate_woe, so the rule is written in the selector too (see "Needs decision").confirm_variablesis now validated at init viasuper().__init__(confirm_variables), like the other selectors.strategygets theisinstance(..., str)check before the membership test.Benchmarks
Machine under load from other jobs during some runs, so I repeated the runs when load was low (load average about 3–5). Numbers below are from those runs. Median of 3 subprocess runs, each the median of 3 fits, alternating old and new. Data: 12 categories per categorical variable, normal numerical variables, binary target,
bins=5.Whole
fit(): base (#1070) vs this PRMost of the remaining time in
fitis spent in shared helpers (_select_all_variables' datetime check, the NaN/inf checks, the discretiser fit).IV aggregation only (numerical variables already discretised), median of 7, rotating order
WoE._calculate_woeper variable, then numpy sumy.groupby(X[var], observed=True, sort=False).agg(["sum", "size"])X[var].factorize()+np.bincount(chosen for pandas)pl.collect_allof one lazy query per variablecollect_alland 1.7x–2.7x faster than a per-variable eager loop, so polars uses the narwhals path.Discretising into integer codes instead of interval labels: pandas 500k 25 numerical variables 0.377s → 0.210s, polars 2M 25 variables 2.216s → 0.840s.
Behaviour
I compared outputs on 37 cases (credit-approval data from the user guide, synthetic data with every parameter branch, NaN, inf, datetime, bool, constant, integer column names, pandas category dtype, non-default index, list/array/str/bool/{1,2} targets, zero-count categories, empty intervals, reordered columns). Old versions compared: v1.9.4, base (#1070), and this PR on pandas and polars.
0.6006252129425703to0.6006252129425705.information_values_values are now Pythonfloaton both backends. Before, pandas gavenp.float64, which is afloatsubclass.Differences vs v1.9.4 (all already present on base, through the
WoEchanges onnarwhals-migration)ValueError: The proportion of one of the classes for a category in variable X is zero...when a category or interval had no positive or no negative cases. Now the zero count is replaced by 0.5 and the IV is computed. This affects many realistic inputs: the credit-approval data with default parameters,bins=10, numerical variables whose outer equal-width intervals hold only a few observations, and any rare category. In v1.9.4 the selector never accepted afill_value-style option: it always passedfill_value=Noneto_calculate_woe, so it always raised. There is nothing to deprecate on the selector's API.observed=False, so an unused category had 0/0 counts and raised the error above. Now unused categories are ignored, and the IV equals the IV of the same column without them.WoE._check_fit_input): in v1.9.4, a target that wasn't 0/1 (for example 1/2 or strings), used with a dataframe with a non-default index, lost its index when remapped. All IVs came out 0.0, and every feature was dropped. Now the IVs are correct. This case is covered bytest_target_not_0_1_with_pandas_index.User guide fixes
variables_was shown as 7 variables includingA7. It returns the 6 variables passed.Tests
tests/test_selection/test_information_value.pyis rewritten to the conventions: init tests first (one per error message, including the newconfirm_variablesvalidation), thenmake_dftests on both backends with explicit expected values (including hand-written IV formulas for the regular and zero-count cases and for empty intervals), plus pandas-only tests for integer column names, category dtype with unused categories, and a non-default index. I removed the test of the private_calculate_ivmethod along with the method; the IV formula is now tested throughfit.When I run the new test file against the base code, only the 5
confirm_variablesvalidation tests fail.pytest tests/test_selection:SelectByInformationValue: they belong to selectors that haven't been migrated yet, and to the sklearncheck_estimatortests for those selectors.flake8 feature_engine tests: clean.mypy feature_engine: 2 errors, the same as base.Needs decision
Zero counts (behaviour change vs 1.9.4, inherited from the
WoEchange). I kept what base does: replace by 0.5, don't raise. The docstring and user guide describe this. Options:variables_with_zero_counts_attribute, asWoEEncoderhas, so users can see which variables were affected.Also: should the user guide get a "New in version 2.0" note about this change, as the
WoEEncoderguide has?Zero-count rule written twice. The 0.5 rule is now in
WoE._calculate_woeand in the selector's IV computation, because reusing_calculate_woeper variable was 2x–14x slower on pandas and about 3x slower on polars than computing the IV directly ("current" column above). If you prefer one place, the IV computation could move to theWoEmixin. That would touchwoe.py, which is outside this PR._check_variable_number(): the addendum says to call it after choosing the variables, but this selector never did, and v1.9.4 accepts a single variable. I didn't add it, because doing so would be a behaviour change.Pre-existing issues, not fixed
BaseSelector.transformcallsnw_X.select(nw.col(*features))with an empty list when every feature is dropped. polars then raisesTypeError: Col.__call__() missing 1 required positional argument: 'name', while pandas returns a dataframe with no columns. This happens easily with this selector (for examplethreshold=1). The fix belongs inbase_selector.py(Migrate the selection base classes and helpers to narwhals, add polars support #1070): guard the empty list, or raise a clear error asDropConstantFeaturesdoes.bins=Trueandthreshold=Truepass the init checks, becauseboolis a subclass ofint._more_tagscomment explaining_skip_testsays the transformer raises on zero counts, which is no longer true. I left the tag alone. Whether the sklearn checks now pass without it is worth a separate look.