Migrate LogTransformer/LogCpTransformer to narwhals, add polars support - #1008
Merged
Conversation
LogTransformer's C parameter (scalar/dict/"auto") makes this more than a
pure elementwise op like the other sibling transformers: "auto" needs a
per-variable min reduction, and the shift C can vary per column. Extract
the transform columns to a single numpy array via narwhals' to_numpy(),
compute the per-column shift with np.where(mins > 0, 0, abs(mins) + 1) for
"auto", broadcast a dict C_ into a numpy array ordered to match
variables_, apply np.log/np.log10 once, reassign via nw.new_series +
with_columns. LogCpTransformer is a subclass of LogTransformer (same file,
no separate work needed).
Benchmarked narwhals-on-pandas vs the old pandas-native .loc-assignment
across 10k-100k rows and 1-10 columns: narwhals-on-pandas ran at 0.53x-0.87x
of the old runtime (avg 0.67x, ~1.5x faster), narwhals-on-polars faster
still (avg 0.59x, ~1.7x faster) - consistent with every other sibling in
this module, so no pandas/polars branch was added; transform() and
inverse_transform() share one merged narwhals path.
Verified pandas/polars parity directly (C=int/dict/"auto", both bases,
including inverse_transform) since pyarrow isn't installed in this env, so
narwhals' to_pandas()/to_native() round-trips weren't usable for
comparison - compared to_dict(as_series=False) output instead.
Rewrote test_log_transformer.py and test_logcp_transformer.py to one
parametrized test per behavior over pandas/polars input (previously
pandas-only, relying on the global df_vartypes/df_na fixtures), replaced
with local DATA/DATA_NA/DATA_C dicts, same pattern as
test_reciprocal_transformer.py. All expected values recomputed and
verified against actual output.
Found one doc/output drift caused by the migration itself: LogCpTransformer.rst
showed `{'MedInc': 0, 'HouseAge': 0}` for C="auto" on strictly-positive
variables, but casting the whole numpy array to float (needed for the
mixed positive/non-positive np.where computation) means the "no shift
needed" case is now 0.0, not int 0 - updated the doc to match. Cosmetic
only: dict equality (0.0 == 0) means no test assertion needed updating.
Verified every other code example already in LogTransformer.rst and
LogCpTransformer.rst against current output (fetch_california_housing/
load_diabetes - no network needed, both ship with scikit-learn) - all
matched exactly. Added a verified "With polars" section to each doc.
flake8/mypy clean on feature_engine/transformation/log.py; sphinx-build -W
clean (only the pre-existing linkcode_resolve warning, unrelated); log.py
imports standalone with pandas import blocked at the builtins level; full
tests/test_transformation suite shows the same 8 pre-existing failures as
the pre-migration baseline (numpy-array input rejected by check_X, a
base-branch issue in dataframe_checks.py predating this work, unrelated
to log.py) and zero new failures.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
solegalli
commented
Aug 25, 2026
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.
LogTransformer's C parameter (scalar/dict/"auto") makes this more than a pure elementwise op like the other sibling transformers: "auto" needs a per-variable min reduction, and the shift C can vary per column. Extract the transform columns to a single numpy array via narwhals' to_numpy(), compute the per-column shift with np.where(mins > 0, 0, abs(mins) + 1) for "auto", broadcast a dict C_ into a numpy array ordered to match variables_, apply np.log/np.log10 once, reassign via nw.new_series + with_columns. LogCpTransformer is a subclass of LogTransformer (same file, no separate work needed).
Benchmarked narwhals-on-pandas vs the old pandas-native .loc-assignment across 10k-100k rows and 1-10 columns: narwhals-on-pandas ran at 0.53x-0.87x of the old runtime (avg 0.67x, ~1.5x faster), narwhals-on-polars faster still (avg 0.59x, ~1.7x faster) - consistent with every other sibling in this module, so no pandas/polars branch was added; transform() and inverse_transform() share one merged narwhals path.
Verified pandas/polars parity directly (C=int/dict/"auto", both bases, including inverse_transform) since pyarrow isn't installed in this env, so narwhals' to_pandas()/to_native() round-trips weren't usable for comparison - compared to_dict(as_series=False) output instead.
Rewrote test_log_transformer.py and test_logcp_transformer.py to one parametrized test per behavior over pandas/polars input (previously pandas-only, relying on the global df_vartypes/df_na fixtures), replaced with local DATA/DATA_NA/DATA_C dicts, same pattern as test_reciprocal_transformer.py. All expected values recomputed and verified against actual output.
Found one doc/output drift caused by the migration itself: LogCpTransformer.rst showed
{'MedInc': 0, 'HouseAge': 0}for C="auto" on strictly-positive variables, but casting the whole numpy array to float (needed for the mixed positive/non-positive np.where computation) means the "no shift needed" case is now 0.0, not int 0 - updated the doc to match. Cosmetic only: dict equality (0.0 == 0) means no test assertion needed updating. Verified every other code example already in LogTransformer.rst and LogCpTransformer.rst against current output (fetch_california_housing/ load_diabetes - no network needed, both ship with scikit-learn) - all matched exactly. Added a verified "With polars" section to each doc.flake8/mypy clean on feature_engine/transformation/log.py; sphinx-build -W clean (only the pre-existing linkcode_resolve warning, unrelated); log.py imports standalone with pandas import blocked at the builtins level; full tests/test_transformation suite shows the same 8 pre-existing failures as the pre-migration baseline (numpy-array input rejected by check_X, a base-branch issue in dataframe_checks.py predating this work, unrelated to log.py) and zero new failures.