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>
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>
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.
Stacked on #1070 (selection base classes). Until #1070 is merged, its commits also show in this diff. The commit for this PR is the last one.
Summary
Migrates
RecursiveFeatureAdditionto narwhals, so it works with pandas and polars:fitusesBaseRecursiveSelector.fit, which returns(nw_X, y). Each round's model is trained on a native frame:X[features]for pandas, andnw_X.select(nw.col(*features)).to_native()for other backends. The cross-validation call is shared by the baseline model and the loop in a small_cross_validatemethod.Series.sort_values, becausefeature_importances_is a dict for polars. It uses numpy with exactly the same ordering assort_values(ascending=False), including for tied importances (see Behaviour). The attribute is then rebuilt with_importance_series, so it stays a pandas Series for pandas input and is a dict otherwise, as in the base class.transformis inherited fromBaseSelector.fitno longer says "pandas dataframe", and there is a polars example. The sharedfeature_importances_/feature_importances_std_attribute docstrings in_docstrings/fit_attributes.pynow describe the Series-or-dict container, using the same wording asBaseRecursiveSelector. RFE uses these docstrings too.initial_model_performance_, and the drift values, which now print asnp.float64(...)with numpy 2.Benchmarks
Cross-validation takes almost all of the fit time. The selector's own work is building the frame for each round (one per feature, with a growing list of columns) and ranking the importances. Times are medians, in ms. Other jobs were running on the machine, so the whole-fit numbers are noisy.
Column subsetting, summed over all rounds of one fit:
select(nw.col(...)).to_native()X[cols]/ polarsX.select(cols))to_numpy()Whole fit (LinearRegression, cv=3):
X.select)X.select)X.select)X[cols]is 2-10x faster than narwhals for the subsetting, and the whole fit is about 10% faster at 10k-100k rows. At 500k rows the whole fit is dominated by noise, in both directions. pandas keeps its own path.selectis 10-25 ms faster per fit than narwhals, but a fit takes 5-20 s. That difference is below the whole-fit noise, which goes both ways in the table. So polars uses the narwhals path, and there is no third branch.sort_values, whether or not the Series is rebuilt afterwards. Both are negligible, so one numpy path is used for both backends.Behaviour
mainfor 18 pandas cases: RF / logistic regression / Lasso / decision tree / linear regression; int, KFold and generator CV; GroupKFold with groups; threshold keeping all and dropping all; a variables subset;confirm_variables; a categorical column; KNN (permutation importance); NaN with HistGradientBoosting; integer column names; and 25 features with tied importances (Lasso and a shallow tree). For each case I comparedfeatures_to_drop_,performance_drifts_(std_),feature_importances_(std_)including their order,initial_model_performance_, andtransformon reordered columns. All 18 are bit-identical on this branch.feature_importances_(std_): a dict for polars, as decided in Migrate the selection base classes and helpers to narwhals, add polars support #1070.sort_valuesuses quicksort, which is not stable for more than 16 values. So with many features, tied features are not ranked in column order. A stable Pythonsorted()would have changed the ranking, and so the selection, for any model that gives several features the same importance (for example Lasso zeros or unused tree features). The numpy code reproduces pandas' ranking exactly, andtest_ranking_of_features_with_equal_importancecovers it.cross_validateis no longer called withreturn_estimator=True, because the fitted estimators were never used. Scores are unchanged.Tests
test_recursive_feature_addition.pyis rewritten to the conventions: init error tests with the full message,test_init_param_assignment, and one test per behaviour on pandas and polars throughmake_df. Targets are built withmake_series, with one test for list and numpy targets. Expected values are explicit, floats are compared withpytest.approx, and outputs are checked withisinstance+frame_to_dict. The diabetes data now has its real column names, so it runs on polars. The pandas-only tests cover integer column names and the Series type of the importances.test_recursive_feature_selectors.py: I removed only the RFA parts, which are now covered in the RFA file: RFA from_selectors, and the RFA blocks of the combined tests. The RFE PR is migrating RFE in parallel and edits the same file. The two PRs touch neighbouring lines, so whichever merges second may need a small rebase.tests/test_selection, one file per pytest call: 137 onnarwhals-selection-base, 123 on this branch. No new failures. The 8 old RFA tests, the 2 RFAtest_fit_initial_model_performancecases and the 4 RFA check_estimator cases now pass.tests/parametrize_with_checks_selection_v16.pyhas the same result before and after: 289 failed, 204 passed.flake8 feature_engine testsis clean.mypy feature_enginegives the same 2 errors as the base, none in this module.Needs decision
feature_importances_/feature_importances_std_are a pandas Series for pandas input and a dict for polars. The user guide's plotting example usespd.concaton the Series, so it only works with pandas.