Conversation
Compute the target mean per bin and per category directly instead of through a Pipeline of discretiser and MeanEncoders, reusing the fitted discretiser's bin edges and labels. Fit and _predict accept pandas and polars dataframes, and pandas integer column names now work. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Drop the pandas import and type fit and predict for any narwhals-supported dataframe, with the target as a series, numpy array or list. Rewrite the tests to run on pandas and polars, and add a polars example to the docstring. 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 #1066 (
BaseTargetMeanEstimatormigration), which is not merged yet: until it is, this diff also shows #1066's changes. The only commit of this PR is the last one.Summary
TargetMeanRegressorno longer imports pandas.fitandpredictare typed for any narwhals-supported dataframe, andfittakes the target as a series, numpy array or list. All the computation lives in the base class (Migrate BaseTargetMeanEstimator to narwhals, add polars support #1066); the regressor only adds the binary-target check (type_of_target) andpredict/score._predictionmodule is private), so no docs page changed.make_df,make_series, full-messagematch=, explicit expected values).Benchmarks
The regressor's own work is
type_of_target(y). I compared it on the native target withtype_of_target(np.asarray(y)), and against the wholefit(median of 7 / 5 runs, ms, float target, half numerical and half categorical columns):The check is under 2% of
fiton both backends and converting to numpy first saves at most ~1 ms, so I kepttype_of_targeton the target as passed (no branch needed).Behaviour
score,encoder_dict_,binner_dict_and error messages over 160 cases (both strategies, bins 1/3/10, 4 variable selections, series/list/numpy targets, binary target, NaN target, short target, unseen category, NaN in X, not fitted, reordered columns, integer column names, non-dataframe input) before and after: all equal.Tests
tests/test_prediction: 122 passed before, 160 passed after, 0 failures.tests/test_selection(importsTargetMeanRegressor): 177 failed / 251 passed before and after, same failing tests (the selection module is not migrated yet).flake8 feature_engine testsclean;mypy feature_enginehas the same 2 errors as the base ref (datetime_subtraction.py,log.py).test_classifier_numerical_variablescheckednum_var_Bwithnp.array_equal(...)withoutassert; the new test asserts it.Needs decision
Xandyfails infit, inadd_target_to_X(shared helper, used by the base class): pandasXwith a polarsyraisesAttributeError: PolarsSeries has not attribute '_broadcast', and polarsXwith a pandasyraises a narwhalsComputeErrorabout a duplicate'literal'column. Lists and numpy arrays work with both. Should mixed backends be supported (convertyto numpy when its backend differs fromX's), or rejected with a clear error? I did not change the helper or the base here.df_regressionfixture intests/test_prediction/conftest.pyis no longer used (the data now lives in the regressor's test file, as it is used by one file only). I left it to avoid a conflict with the parallelTargetMeanClassifierPR, which edits the same fixture file; it can be removed after that one merges.