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>
fit() validates X with check_X and hands the native dataframe to the variable, missing-value and correlation helpers, so pandas keeps its fast paths in find_correlated_features and polars is supported. The tests are rewritten to run on pandas and polars, and the user guide gets a polars example. 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 commits show in this diff until #1070 merges. This PR's own commit is the last one.
Summary
DropCorrelatedFeatures.fit()validatesXwithcheck_X(X)and passes the native dataframe to_select_numerical_variables,_check_contains_na/_check_contains_inf,find_correlated_featuresand_get_feature_names_in. The correlation work happens infind_correlated_featuresfrom Migrate the selection base classes and helpers to narwhals, add polars support #1070.transformis inherited fromBaseSelector.fitrebindsX = check_X(X), so pandas input reachedfind_correlated_featuresas a narwhals frame and took the non-pandas paths. Passing the native frame lets pandas use its own paths again. In particular, spearman with missing values, kendall and callables go back toDataFrame.corr(), and the error for an unknownmethodis pandas'ValueErroragain, as on main.import pandasis removed. The type hints are nowIntoDataFrame/IntoSeries.missing_valuesinit check now checksisinstance(missing_values, str)before the membership test, per AGENTS.md. The allowed values and the message are unchanged.fitparameters are backend-neutral; the intro explains that each pair is compared on the rows where both have values. Attribute descriptions are fixed:features_to_drop_is a list, not a set;correlated_feature_sets_holds sets;correlated_feature_dict_referred to a nonexistentcorrelated_feature_groups.Benchmarks
The selector itself only wires the steps together, so the comparison is between the whole
fit()/fit_transform()on the base ref (a narwhals frame handed to the helpers) and on this PR (the native frame). Times are the median of 5 alternating runs in ms (1 run for kendall and for spearman with NaN). Data has correlated columns, and "NaN" means 5% missing values.pandas
polars (the same code path in both versions: the helpers convert with
nw.from_nativeeither way)The native frame wins or ties everywhere, except pandas spearman without NaN. There, the base ref's accidental path (narwhals
rankon the pandas backend) is 3-4% faster than the scipyrankdatathatfind_correlated_featurespicks for pandas. That choice belongs to the base file, so this PR doesn't change it (see Needs decision). Spearman with NaN is 6-9x faster on pandas becauseDataFrame.corr()is used again.Behaviour
main(pandas) for 64 cases. The cases cover the 4 methods (pearson, spearman, kendall, a callable) at 4 thresholds, with and without NaN, plus mixed dtypes (categorical, int, constant, negatively correlated), reordered columns, andmissing_values="raise"with NaN and with inf. Also covered:variablessubsets,confirm_variables, unknown and non-numerical variables, fewer than 2 variables, an unknown method, thresholds 0.0 and 1.0, transform with reordered columns and with a different number of columns, empty/all-NaN/two-row inputs, integer column names with a non-default index, and a named columns index.get_support,get_feature_names_out, transform values, dtypes, column order and index. The exception is the empty-dataframe error message, which comes fromcheck_Xonnarwhals-migrationand not from this PR.ColumnNotFoundErrorinstead ofKeyError, already listed in Migrate the selection base classes and helpers to narwhals, add polars support #1070;method:TypeError: 'str' object is not callableinstead of pandas'ValueError(see Needs decision).Tests
tests/test_selection/test_drop_correlated_features.pyis rewritten to the conventions:# init parameters: one error test per message (threshold, missing_values, confirm_variables) with wrong values and wrong types, andtest_init_param_assignment.# fit and transform: every test runs on pandas and polars viamake_df, with data as dicts andframe_to_dictcomparisons.variablessubsets,confirm_variables, and the NaN, inf, <2 variables, non-numerical and non-dataframe errors.assert_frame_equal), and the unknown-methoderror, whose message comes from pandas.tests/test_selection, run one file at a time: 137 failing on the base ref, 136 on this branch. No new failures. The fixed one is the oldtest_raises_error_when_method_not_permitted, which failed on the base ref because pandas went through the narwhals path; its replacement passes.tests/parametrize_with_checks_selection_v16.py: 289 failing before and after, the same set (numpy-array input checks).flake8 feature_engine testsis clean.mypy feature_engineshows the same 2 errors as the base ref (datetime_subtraction.py,log.py).Needs decision
methodwith polars.methodis not validated in__init__. pandas raises atfitwith pandas'ValueError, while polars raisesTypeError: 'str' object is not callablefromfind_correlated_features. I kept the current behaviour. Proposal: validate in__init__(isinstance(method, str) and method in ["pearson", "spearman", "kendall"]orcallable(method)), with a message ending "Got {method} instead.". That moves the error fromfitto init and changes the pandas message, so it's your call.SmartCorrelatedSelectionhas the same parameter and would need the same change.methodtype hint.find_correlated_features(method: str)in the base file rejects callables for mypy, so the selector keepsmethod: stralthough callables are allowed. Widening it toUnion[str, Callable]needs a change inbase_selection_functions.py.rankon pandas was 3-4% faster than scipyrankdatain the fit-level numbers above. Migrate the selection base classes and helpers to narwhals, add polars support #1070 measured the opposite at the function level, so it may be noise. It's worth a re-check there, not here.Pre-existing issues, not fixed
TypeError: '<' not supported between instances of 'str' and 'int'infit, because the variables are sorted alphabetically. This also happens on main. Fixing it (for example, sorting bystr) could change which feature is kept in existing pipelines.