Skip to content

Migrate LogTransformer/LogCpTransformer to narwhals, add polars support - #1008

Merged
solegalli merged 2 commits into
narwhals-migrationfrom
narwhals-log-transformer
Aug 25, 2026
Merged

Migrate LogTransformer/LogCpTransformer to narwhals, add polars support#1008
solegalli merged 2 commits into
narwhals-migrationfrom
narwhals-log-transformer

Conversation

@solegalli

Copy link
Copy Markdown
Collaborator

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.

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>
Comment thread feature_engine/transformation/log.py Outdated
@solegalli
solegalli merged commit ddbb256 into narwhals-migration Aug 25, 2026
3 of 10 checks passed
@solegalli
solegalli deleted the narwhals-log-transformer branch August 25, 2026 19:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant