Skip to content

Migrate CategoricalMethodsMixin (encoding base) to narwhals, add pola… - #999

Open
solegalli wants to merge 1 commit into
narwhals-migrationfrom
narwhals-encoding-base
Open

Migrate CategoricalMethodsMixin (encoding base) to narwhals, add pola…#999
solegalli wants to merge 1 commit into
narwhals-migrationfrom
narwhals-encoding-base

Conversation

@solegalli

Copy link
Copy Markdown
Collaborator

…rs support

Shared base for all 8 encoders. _get_feature_names_in() and _check_transform_input_and_state() follow the same is_pandas-gated column-reorder pattern as BaseImputer/DecisionTreeFeatures. _check_or_select_variables() needed no change: the variable_handling helpers it calls are already fully narwhals-generic.

The hot path is _encode()/inverse_transform(), a per-column dict-based map applied on every transform() call across every encoder. Benchmarked pandas-native .map(dict) vs narwhals Series.replace_strict(dict, default=...) at 10k/50k/100k rows x 1/2/10 columns x 5/50 categories (warmed up first to remove first-call JIT/import overhead): narwhals-on-pandas lands at ~1.06x-1.2x of pandas-native at realistic sizes (50k-100k rows), i.e. minimal loss - merged into a single narwhals path per the established decision rule, no pandas fast-path split. narwhals-on- polars is consistently ~4-5x faster than pandas-native at 100k rows.

replace_strict() also simplifies the old logic: pandas' plain .map() leaves category-dtype columns as category dtype after mapping, which the old code corrected with a manual "cast to int if all-int else float" step. Verified narwhals' replace_strict resolves straight to a plain numeric dtype on both a pandas category column and a polars Categorical column, so that dtype fixup is dead code once replace_strict replaces .map() - dropped it entirely rather than porting it.

Used Series.get_column().replace_strict() (not nw.col(), which only accepts string names) throughout, same as DecisionTreeFeatures' precedent for pandas integer column names - nw.col(feature) blew up on int-named columns (caught by the existing
test_column_names_are_numbers test, which polars can't cover since it has no integer-column-name concept).

_check_nan_values_after_transformation() rewritten off pandas' .isnull().sum().sum()/.columns[...] chain onto per-column Series.null_count(), for the same int-column-name reason.

Verified: tests/test_encoding full suite unchanged (17 pre-existing failures - numpy-array-input rejection per the narwhals check_X() contract, plus 3 MeanEncoder inverse_transform failures caused by a pre-existing bug in mean_encoding.py's still-unmigrated fit() passing a numpy y into y.groupby(); reproduced identically against the unmodified base_encoder.py to confirm neither predates nor is introduced by this change - 326 passed both before and after, same failing test IDs). flake8 and mypy clean on the file. Module imports with pandas blocked (loaded standalone, since sibling encoder files in this package are not yet migrated and still import pandas at their own module level). sphinx -W build clean (only the pre-existing unrelated linkcode_resolve warning). Manually verified CountEncoder end-to-end on polars input (fit still pandas-only until its own migration, transform/inverse_transform now backend-agnostic via this mixin) produces identical values to the pandas path, including a pre-existing quirk where count-encoding inverse_transform is ambiguous for categories that share a count (confirmed identical, not a regression, on the old code too).

_helper_functions.py checked: pure-python parameter validation, no dataframe interaction, no pandas import - left untouched.

…rs support

Shared base for all 8 encoders. _get_feature_names_in() and
_check_transform_input_and_state() follow the same is_pandas-gated
column-reorder pattern as BaseImputer/DecisionTreeFeatures.
_check_or_select_variables() needed no change: the variable_handling
helpers it calls are already fully narwhals-generic.

The hot path is _encode()/inverse_transform(), a per-column
dict-based map applied on every transform() call across every
encoder. Benchmarked pandas-native .map(dict) vs narwhals
Series.replace_strict(dict, default=...) at 10k/50k/100k rows x
1/2/10 columns x 5/50 categories (warmed up first to remove
first-call JIT/import overhead): narwhals-on-pandas lands at
~1.06x-1.2x of pandas-native at realistic sizes (50k-100k rows),
i.e. minimal loss - merged into a single narwhals path per the
established decision rule, no pandas fast-path split. narwhals-on-
polars is consistently ~4-5x faster than pandas-native at 100k rows.

replace_strict() also *simplifies* the old logic: pandas' plain
.map() leaves category-dtype columns as category dtype after
mapping, which the old code corrected with a manual "cast to int
if all-int else float" step. Verified narwhals' replace_strict
resolves straight to a plain numeric dtype on both a pandas
category column and a polars Categorical column, so that dtype
fixup is dead code once replace_strict replaces .map() - dropped
it entirely rather than porting it.

Used Series.get_column().replace_strict() (not nw.col(), which
only accepts string names) throughout, same as DecisionTreeFeatures'
precedent for pandas integer column names - nw.col(feature) blew up
on int-named columns (caught by the existing
test_column_names_are_numbers test, which polars can't cover since
it has no integer-column-name concept).

_check_nan_values_after_transformation() rewritten off pandas'
.isnull().sum().sum()/.columns[...] chain onto per-column
Series.null_count(), for the same int-column-name reason.

Verified: tests/test_encoding full suite unchanged (17 pre-existing
failures - numpy-array-input rejection per the narwhals check_X()
contract, plus 3 MeanEncoder inverse_transform failures caused by a
pre-existing bug in mean_encoding.py's still-unmigrated fit() passing
a numpy y into y.groupby(); reproduced identically against the
unmodified base_encoder.py to confirm neither predates nor is
introduced by this change - 326 passed both before and after, same
failing test IDs). flake8 and mypy clean on the file. Module imports
with pandas blocked (loaded standalone, since sibling encoder files
in this package are not yet migrated and still import pandas at
their own module level). sphinx -W build clean (only the
pre-existing unrelated linkcode_resolve warning). Manually verified
CountEncoder end-to-end on polars input (fit still pandas-only until
its own migration, transform/inverse_transform now backend-agnostic
via this mixin) produces identical values to the pandas path,
including a pre-existing quirk where count-encoding inverse_transform
is ambiguous for categories that share a count (confirmed identical,
not a regression, on the old code too).

_helper_functions.py checked: pure-python parameter validation, no
dataframe interaction, no pandas import - left untouched.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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