Migrate BoxCoxTransformer to narwhals, add polars support - #1006
Merged
Conversation
fit()'s lambda search is per-column and not vectorizable (scipy.stats.boxcox with lmbda=None does per-column MLE optimization), while transform()/ inverse_transform() are pure elementwise math once lambdas are known - scipy.special.boxcox/inv_boxcox are ufuncs that broadcast a per-column lambda array against a 2D values array, so both methods extract via narwhals' to_numpy() once and apply a single batched call, same precedent as PowerTransformer (merged, not split). Benchmarked pandas-native vs narwhals-on-pandas vs narwhals-on-polars at 10k/50k/100k rows x 1/2/10 columns, fit and transform measured separately since they're different cost centers: - fit(): scipy's lambda-search optimization dominates total cost by 2-3 orders of magnitude over transform() (e.g. 10k rows/1 col: ~12.6ms fit vs ~0.1ms transform). narwhals overhead there is noise (<1% at every size/column combination tested). - transform(): narwhals-on-pandas adds a small absolute overhead at tiny sizes (10k rows/1 col: 0.10ms old vs 0.27ms narwhals-loop/0.27ms narwhals-batched) but this shrinks to parity or better by 100k rows (9.03ms old vs 8.90ms narwhals-batched-pandas). Given fit() so overwhelmingly dominates real-world cost, a pandas/polars split for transform() would be real complexity for no measurable benefit - merged into a single narwhals path for both methods, matching every sibling transformer migrated in this module so far. Rewrote test_boxcox_transformer.py to one parametrized test per behavior over make_df=[pd.DataFrame, pl.DataFrame], replacing the pandas-only df_vartypes/df_na fixtures with local DATA/DATA_NA dicts (same convention as test_relative_features.py). All expected values verified against actual output on both backends - identical. docs/user_guide/transformation/BoxCoxTransformer.rst's main walkthrough uses fetch_openml against the Ames house-prices dataset; this sandbox has no network access (SSL/DNS blocked), so that section's numbers are UNVERIFIED against current output - flagging per instructions rather than silently skipping. Added a fully-verified "With polars" section using simple synthetic data, following the PowerTransformer precedent. Verified: pytest tests/test_transformation (136 passed, same 8 pre-existing check_estimator failures as the unmigrated baseline, none new - confirmed those predate this change and affect all 8 transformers in the module, including ones not yet migrated); flake8 feature_engine tests clean; mypy feature_engine/transformation/boxcox.py clean; sphinx-build -W clean aside from the pre-existing unrelated linkcode_resolve warning (confirmed identical on the unmigrated base branch); boxcox.py and its full import chain load standalone with pandas import blocked. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
solegalli
added a commit
that referenced
this pull request
Aug 30, 2026
fit()'s lambda search is per-column and not vectorizable (scipy.stats.boxcox with lmbda=None does per-column MLE optimization), while transform()/ inverse_transform() are pure elementwise math once lambdas are known - scipy.special.boxcox/inv_boxcox are ufuncs that broadcast a per-column lambda array against a 2D values array, so both methods extract via narwhals' to_numpy() once and apply a single batched call, same precedent as PowerTransformer (merged, not split). Benchmarked pandas-native vs narwhals-on-pandas vs narwhals-on-polars at 10k/50k/100k rows x 1/2/10 columns, fit and transform measured separately since they're different cost centers: - fit(): scipy's lambda-search optimization dominates total cost by 2-3 orders of magnitude over transform() (e.g. 10k rows/1 col: ~12.6ms fit vs ~0.1ms transform). narwhals overhead there is noise (<1% at every size/column combination tested). - transform(): narwhals-on-pandas adds a small absolute overhead at tiny sizes (10k rows/1 col: 0.10ms old vs 0.27ms narwhals-loop/0.27ms narwhals-batched) but this shrinks to parity or better by 100k rows (9.03ms old vs 8.90ms narwhals-batched-pandas). Given fit() so overwhelmingly dominates real-world cost, a pandas/polars split for transform() would be real complexity for no measurable benefit - merged into a single narwhals path for both methods, matching every sibling transformer migrated in this module so far. Rewrote test_boxcox_transformer.py to one parametrized test per behavior over make_df=[pd.DataFrame, pl.DataFrame], replacing the pandas-only df_vartypes/df_na fixtures with local DATA/DATA_NA dicts (same convention as test_relative_features.py). All expected values verified against actual output on both backends - identical. docs/user_guide/transformation/BoxCoxTransformer.rst's main walkthrough uses fetch_openml against the Ames house-prices dataset; this sandbox has no network access (SSL/DNS blocked), so that section's numbers are UNVERIFIED against current output - flagging per instructions rather than silently skipping. Added a fully-verified "With polars" section using simple synthetic data, following the PowerTransformer precedent. Verified: pytest tests/test_transformation (136 passed, same 8 pre-existing check_estimator failures as the unmigrated baseline, none new - confirmed those predate this change and affect all 8 transformers in the module, including ones not yet migrated); flake8 feature_engine tests clean; mypy feature_engine/transformation/boxcox.py clean; sphinx-build -W clean aside from the pre-existing unrelated linkcode_resolve warning (confirmed identical on the unmigrated base branch); boxcox.py and its full import chain load standalone with pandas import blocked. Co-authored-by: Claude Sonnet 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.
fit()'s lambda search is per-column and not vectorizable (scipy.stats.boxcox with lmbda=None does per-column MLE optimization), while transform()/ inverse_transform() are pure elementwise math once lambdas are known - scipy.special.boxcox/inv_boxcox are ufuncs that broadcast a per-column lambda array against a 2D values array, so both methods extract via narwhals' to_numpy() once and apply a single batched call, same precedent as PowerTransformer (merged, not split).
Benchmarked pandas-native vs narwhals-on-pandas vs narwhals-on-polars at 10k/50k/100k rows x 1/2/10 columns, fit and transform measured separately since they're different cost centers:
Rewrote test_boxcox_transformer.py to one parametrized test per behavior over make_df=[pd.DataFrame, pl.DataFrame], replacing the pandas-only df_vartypes/df_na fixtures with local DATA/DATA_NA dicts (same convention as test_relative_features.py). All expected values verified against actual output on both backends - identical.
docs/user_guide/transformation/BoxCoxTransformer.rst's main walkthrough uses fetch_openml against the Ames house-prices dataset; this sandbox has no network access (SSL/DNS blocked), so that section's numbers are UNVERIFIED against current output - flagging per instructions rather than silently skipping. Added a fully-verified "With polars" section using simple synthetic data, following the PowerTransformer precedent.
Verified: pytest tests/test_transformation (136 passed, same 8 pre-existing check_estimator failures as the unmigrated baseline, none new - confirmed those predate this change and affect all 8 transformers in the module, including ones not yet migrated); flake8 feature_engine tests clean; mypy feature_engine/transformation/boxcox.py clean; sphinx-build -W clean aside from the pre-existing unrelated linkcode_resolve warning (confirmed identical on the unmigrated base branch); boxcox.py and its full import chain load standalone with pandas import blocked.