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>
SelectByShuffling works with pandas, polars and other narwhals-supported dataframes. For a given random_state the shuffles, drifts and selected features are the same as before and the same with every library. The validation folds come from cross_validate(return_indices=True), so each model is scored on the fold it was not trained on, also when the splitter gives different folds on every call. The fold dataframes are built once and one column is shuffled at a time, instead of copying the whole dataframe for every feature. 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). Its commit shows in the diff until #1070 is merged; this PR's own changes are in the last commit.
Summary
SelectByShufflingaccepts pandas, polars and other narwhals-supported dataframes and returns the same library it receives (through the inheritedBaseSelector.transform).pandasis no longer imported.random_state. pandasSeries.sample(frac=1, random_state=rs)callsrs.choice(n, n, replace=False), which isrs.permutation(n). The selector now callsrandom_state.permutation(n)directly, so every column gets the same shuffle as before, whatever the backend.X[variables_]for every feature, shuffled one column, and then sliced every validation fold again. Now the validation-fold dataframes (andyper fold) are built once. For each feature, only the fold's rows of the shuffled column are gathered (column[permutation[idx]]). pandas replaces that column in place in the fold copy and restores it after scoring. narwhals useswith_columnson the fold frame.cross_validate(..., return_indices=True)instead of a secondcheck_cv(...).split(...)(this is a bug fix, see below). TheX/yreset_indexcalls are gone because all indexing is positional (iloc/_safe_indexing).thresholdinit check now follows the conventions:threshold is not None and not isinstance(threshold, (int, float)), with the messagethreshold must be an integer, a float or None. Got {threshold} instead.initial_model_performance_was0.488702767247119, the code returns0.48870212980353145), and fixed two sentences that said features are dropped when their drift is greater than the mean (it's smaller). I ran every example and pasted the real output.Benchmarks
The machine was heavily loaded during the runs (load average 40-65 on 10 cores), so the timings are noisy. BLAS was single-threaded, each number is the median of 9 repeats (whole fit: 5), and the order of the versions alternated. Data: normal floats,
LinearRegression,r2, 3-fold CV. I picked this cheap estimator so the selector's own work is visible. With real models,predictdominates.pandas: shuffle + re-score loop, all features (seconds)
sample+iloc)iloc[:, j]write + restoreSame loop without scoring, which isolates the selector's own work: at 500k x 50 it was 3.25 s for the current code and 1.08 s for setitem + restore (100k x 50: 0.63 vs 0.11). All strategies gave identical drifts (asserted in the benchmark).
pandas: whole
fit()(seconds, two alternating runs)Why setitem + restore over the
iloc[:, j]in-place write: the two were within noise in the whole fit. Theilocwrite keeps the fold frame as one block, so it was slightly faster in the scored loop. But it needs the column position, and it relies on in-place writes that behave differently between pandas 2.2 and pandas 3 (copy-on-write) and with extension dtypes. Replacing the column works the same everywhere and keeps nullable dtypes (.array.take).polars: shuffle + re-score loop (seconds)
gatherpl.Seriesshuffle(seed), different resultsnarwhals, native polars
gatherand numpy were within noise of each other, so the code keeps the narwhals path with no native polars branch. polars' ownshuffleis faster (see "Needs decision"). Wholefit()on polars: 500k x 10: 0.84 s, 500k x 50: 4.57 s, 2M x 10: 2.15 s, 2M x 30: 6.90 s.Behaviour
Compared with the pandas code on
mainover 22 scenarios: RF / logistic / HistGradientBoosting classifiers, linear and tree regressors,roc_auc/accuracy/r2/neg_mean_squared_error, threshold set /None/0, int / splitter / generator / shuffledKFoldcv, avariablessubset,confirm_variables, categorical columns, reordered columns, NaN, nullableInt64, integer column names, a non-default index,yas a Series / one-column DataFrame / list / array,sample_weight, and the docstring example.initial_model_performance_, everyperformance_drifts_andperformance_drifts_std_value (compared exactly, not approximately),features_to_drop_,variables_,feature_names_in_and thetransformoutput all match.performance_drifts_/performance_drifts_std_are dicts for every backend, as before.Bug fixed
cross_validateand then calledcheck_cv(cv).split(X, y)again to get the validation folds. With a splitter that gives different folds on each call, the models were scored on rows they had been trained on. Examples areKFold(shuffle=True)withoutrandom_state,ShuffleSplit(), or a splitter seeded with aRandomStateinstance. The folds now come fromcross_validate(return_indices=True). Test:test_performance_is_evaluated_on_the_folds_of_each_model. It uses a splitter that alternates its folds, and the same comparison fails onmain. Deterministic splitters (the default) give the same results as before.thresholdvalidation. Falsy invalid values ("",[]) slipped through the oldif threshold and ...check and then silently meant "use the mean". They now raise, with the conventional message.Tests
tests/test_selection/test_shuffle_features.pyis rewritten to the conventions:confirm_variables), andtest_init_param_assignment.make_dfand thedata_classificationfixture: fit attributes, transform, the mean-drift threshold, regression withr2, andneg_msewith a DataFrame target. Also cv as int / splitter / generator, the fold bug, non-numerical columns ignored,confirm_variables, missing values,sample_weight, list and array targets, and a check that the input is not modified.Int64.All expected values were checked against the pandas code on
main.tests/test_selectionfailing tests, base (narwhals-selection-base) vs this branch: 137 → 126, no new failures. The fixed ones are 6 oldtest_shuffle_featurestests plus 5test_check_estimator_selectorschecks forSelectByShuffling.flake8 feature_engine testsis clean.mypy feature_enginegives the same 2 errors as the base (datetime_subtraction, log).Needs decision
Series.shuffle(seed)cuts the selector's own shuffle work by about 40-45% at 500k-2M rows (for example 1.52 s → 0.90 s at 2M x 30 without scoring). Most of the remaining cost is numpy's legacyRandomState.permutation(about 14 ms per column at 500k rows, vs about 8.6 ms for polarsshuffle). But polars' generator gives different shuffles, so polars results would no longer match pandas for the samerandom_state, and it would need an int seed drawn fromrandom_state. A numpyGenerator(about 6 ms) would also be faster, but it changes the pandas results too. With any non-trivial estimator,predictdominates the run time, so I kept identical results. Switch?threshold=0means "use the mean drift". The check isif not self.threshold:, sothreshold=0is treated likeNone. That looks like a bug (someone passing 0 probably wants "drop features whose drift is negative"), butSelectBySingleFeaturePerformanceandSelectByTargetMeanPerformanceuse the same pattern, so I kept the behaviour for consistency. Fix it in all three (is None) in a follow-up?Pre-existing issues, not fixed
sample_weightis used to train the models but not to score them (neither the initial score nor the shuffled ones). Same as before.