Add shared backend test fixtures and helpers for narwhals-migrated tests - #1045
Merged
Merged
Conversation
…r tests
Tests for narwhals-migrated transformers each defined their own way of
building pandas/polars inputs and reading results back (_to_backend,
_assert_values, _cols, _to_dict, _to_pandas, ...), which makes the test
suite hard to maintain. Standardise on one structure:
- tests/conftest.py: `make_df` fixture parametrized over pd.DataFrame and
pl.DataFrame (ids "pandas"/"polars"). Tests that request it run once per
backend; pandas-only tests simply don't request it.
- tests/backend_helpers.py: `to_dict` (contents as {column: values}, NaN
normalised to None), `null_count`, and `make_series` (target on the same
backend as X).
- tests/test_encoding/conftest.py: data shared by the encoder tests, as
fixtures returning plain dicts (missing values written as None) that tests
build with make_df(data).
CountEncoder tests are migrated to this structure: they check the output is
of the input backend with isinstance(X, make_df) and compare contents with
to_dict().
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Shared by the OneHotEncoder, RareLabelEncoder and StringSimilarityEncoder tests, which each defined their own copy of this data. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This was referenced Sep 15, 2026
solegalli
added a commit
that referenced
this pull request
Sep 15, 2026
The shared helper was renamed from to_dict to frame_to_dict in #1045. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
solegalli
added a commit
that referenced
this pull request
Sep 15, 2026
…gData output type (#1046) * Use shared backend test fixtures and helpers in imputation tests Move the data duplicated across the imputer test files into tests/test_imputation/conftest.py (data_na, and data_na_dob for the two transformers that need a never-null datetime column), and replace the file-local helpers (_cols, _null_count, _values, _same_values, assert_df_equal, _missing_count, _to_list, _make_series) with the shared ones: make_df fixture, to_dict, null_count and make_series. Every transform output is now also checked to be of the input backend with isinstance(X, make_df). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Fix DropMissingData returning a narwhals frame and skipping its pandas path _select_rows() receives the narwhals frame returned by check_X, but still dispatched with nwd.is_pandas_dataframe(X), which is never True for a narwhals frame (narwhals warns about it). As a result: - when no variable was selected (e.g. missing_only=True on a clean training set), transform() returned the narwhals frame itself instead of a pandas/polars dataframe; - the benchmarked pandas fast path never ran, so pandas input silently went through the slower narwhals expression path. Branch on X.implementation.is_pandas() instead, and always return the native frame. Caught by the new isinstance(X, make_df) output checks. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Use frame_to_dict in imputation tests The shared helper was renamed from to_dict to frame_to_dict in #1045. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * update conftest * update arbitrary imputer tests * update categorical imputer * reorder tests in drop missing data * refactor end tail tests * refactor mean median imputer tests * refactor random sampler and improve seeding procedure --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
solegalli
added a commit
that referenced
this pull request
Sep 15, 2026
The shared helper was renamed from to_dict to frame_to_dict in #1045. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
solegalli
added a commit
that referenced
this pull request
Sep 15, 2026
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
solegalli
added a commit
that referenced
this pull request
Sep 15, 2026
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
solegalli
added a commit
that referenced
this pull request
Sep 15, 2026
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
solegalli
added a commit
that referenced
this pull request
Sep 15, 2026
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
solegalli
added a commit
that referenced
this pull request
Sep 15, 2026
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
solegalli
added a commit
that referenced
this pull request
Sep 15, 2026
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
solegalli
added a commit
that referenced
this pull request
Sep 15, 2026
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
solegalli
added a commit
that referenced
this pull request
Sep 15, 2026
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
solegalli
added a commit
that referenced
this pull request
Sep 15, 2026
* Migrate OrdinalEncoder.fit() to narwhals, add polars support fit() has two paths: "arbitrary" (X[var].unique()) and "ordered" (target mean per category, via y.groupby(X[var])). transform() and inverse_transform() already came dataframe-agnostic for free from CategoricalMethodsMixin (base_encoder.py, merged separately). Benchmarked a pure-narwhals fit() (group_by/agg/sort for "ordered", unique() for "arbitrary") at 10k-100k rows x 1-10 cols x 5-50 categories: it ran 5x-18x slower than pandas-native fit() at every size tested - a large, consistent loss, unlike the ~1.1x seen for the encode/transform hot path in base_encoder.py. Per the benchmark-driven merge-vs-split rule, this is a real loss, so fit() splits on `is_pandas = nwd.is_pandas_dataframe(X)`: pandas keeps a close variant of its original groupby/unique code (confirmed via a like-for-like full-class benchmark to run within noise of the old code, ~1.0x), while polars (and any other narwhals backend) goes through group_by()/agg()/sort()/unique(). New pandas branch differs from the old code only in how "ordered" pairs y with X[var] (see bug below) - "arbitrary" is untouched. Two real issues found, confirmed against the unmodified pre-migration file (both predate this migration): 1. Bug (fixed): the old "ordered" fit() always called `y.groupby(X[var])`, which raises AttributeError whenever y is a numpy array rather than a Series - e.g. list/array-like y input, which sklearn's check_X_y machinery converts to numpy. This is exactly the scenario tests/test_encoding/test_check_estimator_encoders.py ::test_encoders_when_x_pandas_y_numpy exercises for OrdinalEncoder (encoder2, added in 2022 for issue #376) - it failed against the unmodified file and now passes. Fixed on both the pandas branch (pair X[var] with y via `.assign()`, which aligns a numpy y positionally and a Series y by index, instead of `y.groupby(X[var])`) and the narwhals branch (`nw.new_series` for a numpy y). 2. Cross-backend ordering hazard (avoided, not a regression since old code was pandas-only): grouping by category then sorting by target mean does not, by itself, guarantee the same tie-break order on ties across backends - verified polars reversed two tied categories relative to pandas without it. Old pandas code effectively tie-broke on the category itself (pandas groupby sorts keys ascending by default, and sort_values() is stable). Reproduced that explicitly with a compound sort `.sort([target_name, var])` in the narwhals branch; verified pandas and polars now produce the same dict for a deliberately tied-mean fixture, matching the old code's order exactly. Rewrote every test in test_ordinal_encoder.py as one @pytest.mark.parametrize("make_df", [pd.DataFrame, pl.DataFrame]) case per behavior (43 tests, up from 26), using a narwhals-based, NaN-aware comparison helper. test_variables_cast_as_category stays pandas-only - it exercises pandas Categorical dtype, which polars has no direct equivalent for. Verified: tests/test_encoding/test_ordinal_encoder.py 43 passed. tests/test_encoding full suite: 344 passed, 16 failed - identical failing test IDs to the unmodified base (17 failures, one of which is the bug fixed above), all pre-existing and unrelated to OrdinalEncoder (numpy-X rejection per the narwhals check_X() contract, and MeanEncoder's own unmigrated fit() bug). flake8 and mypy clean. Module imports with pandas blocked. sphinx -W build clean (only the pre-existing linkcode_resolve warning, confirmed identical on the unmodified base). Verified every code example in docs/user_guide/encoding/OrdinalEncoder.rst against real output (California Housing dataset) and added a "With polars" section, verified the same way; the Titanic-dataset examples in that file could not be re-run in this sandbox (no network access to openml.org) but are untouched by this change and were not touched. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Adapt OrdinalEncoder to narwhals-returning check_X check_X / check_X_y now return a narwhals frame, so bind that to nw_X and keep the original native X for _check_or_select_variables, _check_na, _get_feature_names_in and the nwd.is_pandas_dataframe(X) fast-path check (those helpers still expect native input, matching the CategoricalImputer migration on narwhals-migration). The pandas groupby/unique fast path is unchanged - X stays native so no rehydration is needed. The narwhals branch reuses nw_X from check_X / check_X_y instead of nw.from_native(X). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Use shared backend test fixtures and helpers in OrdinalEncoder tests Replace the file-local _to_backend/_assert_values helpers with the shared test structure: make_df and data_enc* fixtures, y built with make_series on the backend under test, isinstance(X, make_df) plus to_dict() checks, and pytest.raises/warns(match=re.escape(msg)). Add a test passing the target as a list and as a numpy array, which take a different code path than a Series. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Use frame_to_dict after the shared helper rename in #1045 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor code * Check encoding_method type, simplify OrdinalEncoder fit, group init tests Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Use add_target_to_X in OrdinalEncoder Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
solegalli
added a commit
that referenced
this pull request
Sep 15, 2026
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
solegalli
added a commit
that referenced
this pull request
Sep 15, 2026
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
solegalli
added a commit
that referenced
this pull request
Sep 15, 2026
* Migrate WoEEncoder to narwhals, add polars support fit() splits by backend: pandas keeps _calculate_woe()'s existing two-groupby implementation unchanged (it's directly unit-tested for that exact pandas-Series-with-category-index contract); polars/other narwhals backends use one group_by() instead of two, deriving the negative-class count as the complement of the positive-class count per category - benchmarked competitive with, and often faster than, pandas-native at 50k-100k rows. Zero-count-per-class fill_value handling preserved exactly. Bug fix: _check_fit_input() previously assumed y was always a pandas Series (y.nunique()/y.min()/y.max()), breaking on a numpy y (e.g. a plain list/array-like target, which sklearn's check_X_y machinery converts via column_or_1d). Wrapped numpy y into a narwhals Series aligned to X's backend; for pandas specifically, also had to line the wrapped Series up with X's actual index, since _calculate_woe()'s y.groupby(X[var]) aligns by index and a mismatched default RangeIndex silently drops every row instead of raising, leaving encoder_dict_ empty. Fixes test_encoders_when_x_pandas_y_numpy's WoEEncoder case (was failing on the unmigrated file, confirmed pre-existing). Verified: 44/44 own tests, full encoding suite 342 passed/16 failed (was 17 pre-existing on the narwhals-encoding-base baseline - one less here since this branch's own numpy-y bug is now fixed, rest confirmed unrelated), flake8 and mypy clean, sphinx -W build clean (only the pre-existing unrelated linkcode_resolve warning). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Adapt WoEEncoder to narwhals-returning check_X check_X_y now returns a narwhals frame. In _check_fit_input, bind that to nw_X and keep the original native X: the nwd.is_pandas_dataframe(X) check, the native_y.index = X.index alignment and the returned X all need native input, and fit()'s pandas _calculate_woe fast path and nwd checks are then unchanged (X stays native so no rehydration is needed). Take the y-series backend from nw_X.implementation instead of re-wrapping X. In transform(), bind _check_transform_input_and_state to nw_X, keep native X for _check_contains_na, and pass nw_X to _encode (which now expects narwhals). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Use shared backend test fixtures and helpers in WoEEncoder tests Replace the file-local data dicts and assert_df_equal/_none_to_nan helpers with the shared test structure: make_df and data_enc* fixtures, y built with make_series on the backend under test, isinstance(X, make_df) plus to_dict() checks, and pytest.raises/warns(match=re.escape(msg)). Add a test passing the target as a list and as a numpy array, which take a different code path than a Series. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Use frame_to_dict after the shared helper rename in #1045 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Use add_target_to_X in WoEEncoder, group init tests, match errors Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Compute WoE with narwhals in _calculate_woe, shared by WoEEncoder and SelectByInformationValue Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Replace zero counts by 0.5 in WoE, remove fill_value, add variables_with_zero_counts_ Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
solegalli
added a commit
that referenced
this pull request
Sep 15, 2026
* Migrate OneHotEncoder to narwhals, add polars support
Uses narwhals' to_dummies() for the actual expansion rather than a manual
numpy/dict loop, since it's a real vectorized one-hot op on both backends.
Handles two edge cases to_dummies() doesn't cover directly: a fixed-length
prefix placeholder ("__ohe_tmp__") swapped back out by slicing rather than
by using the real column name, since to_dummies() only prefixes with the
Series name when it's truthy - a falsy real name (e.g. an int column
literally named 0) would otherwise silently drop the prefix; and learned
categories absent from (or present-but-unlearned in) a given transform
batch, filled with an explicit all-0 column so unseen categories are
encoded as 0 across the board, matching the pre-narwhals behavior exactly.
fit()'s value_counts()/unique() calls and transform()'s reassembly are a
single unified narwhals path - no pandas/polars split needed, verified
directly on both backends (identical dummy columns/values for identical
input).
Rewrote tests/test_encoding/test_onehot_encoder.py to the single
cross-backend-parametrized-test convention: local dict fixtures (dropping
the pandas-only global df_enc_big/df_enc_numeric/df_enc_binary fixtures)
parametrized over make_df in [pd.DataFrame, pl.DataFrame], with narwhals-
based column/sum assertions replacing pd.testing.assert_frame_equal.
test_variables_cast_as_category stays pandas-only (pandas category dtype
has no polars equivalent under test there).
Verified: 43/43 own tests, full encoding suite 340 passed/17 pre-existing
failures (matches the narwhals-encoding-base baseline exactly), flake8
and mypy clean, sphinx -W build clean (only the pre-existing unrelated
linkcode_resolve warning), no pandas import in this file itself.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* Adapt OneHotEncoder to narwhals-returning check_X
Bind check_X / _check_transform_input_and_state results to nw_X and keep
the original native X for _check_or_select_variables, _check_contains_na
and _get_feature_names_in (those helpers still expect native input,
matching the CategoricalImputer migration on narwhals-migration). Drop
the now-redundant nw.from_native(X) round-trips in fit() and transform();
they reuse the narwhals frame returned by check_X /
_check_transform_input_and_state.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* Use shared backend test fixtures and helpers in OneHotEncoder tests
Replace the file-local data dicts and _columns/_colsum helpers with the
shared test structure: make_df and data_enc* fixtures, isinstance(X, make_df)
plus to_dict() checks (keeping the column-order assertions, which are part of
this encoder's output contract), and pytest.raises(match=re.escape(msg)).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* Use frame_to_dict after the shared helper rename in #1045
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* Group OneHotEncoder init tests, match errors, shorten comments
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* Match the fixed get_feature_names_out error message
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* refactor ohe
* fix code style
---------
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
solegalli
added a commit
that referenced
this pull request
Sep 16, 2026
* Migrate RareLabelEncoder to narwhals, add polars support
fit() replaces pandas .unique()/.value_counts(normalize=True) with
narwhals Series.n_unique() (for the cardinality check - matches pandas'
plain unique() length, which counts a null as its own category, unlike
pandas' nunique() which drops it) and drop_nulls().value_counts(sort=True,
normalize=True) (same drop_nulls()/sort=True reasoning as
CountEncoder.fit(): narwhals' value_counts() has no dropna param and
narwhals' own value_counts default is unsorted).
transform() doesn't reuse CategoricalMethodsMixin._encode() (that's a
dict-based numeric remap; this encoder keeps frequent categories as-is
and only replaces the rest), so it's rewritten from pandas'
.loc[~isin(...), feature] = replace_with onto
nw.when(<Series>).then(<Series>).otherwise(nw.lit(replace_with)).alias(
feature). Passing Series from get_column() (not nw.col()) into
when/then/otherwise keeps this working for pandas integer column names,
same as base_encoder.py's precedent. A pandas Categorical column still
needs its own add_categories(replace_with) step before assignment - kept
as a small is_pandas-gated block (structural, like base_encoder.py's
existing reorder branches), since narwhals has no cross-backend
equivalent and polars has no matching restriction. Unlike the old
pandas-only code, no manual object-dtype fixup is needed before
assignment for the ignore_format + numeric-variable + string
replace_with case: narwhals resolves the common dtype itself (object in
pandas, cast-to-string in polars).
Benchmarked pandas-native vs narwhals-on-pandas vs narwhals-on-polars at
10k/50k/100k rows x 1/2/10 columns x 5/50 categories, warmed up. First
pass (zip_with(col, new_series_filled_with_replace_with)) averaged
2.41x pandas-native at 50k-100k rows - most of that cost was
constructing a full same-length replacement Series every transform()
call (~2.5ms of a ~4.8ms transform at 100k rows, confirmed by isolating
just the Series construction). Switched to nw.when(keep).then(col)
.otherwise(nw.lit(replace_with)), which lets the backend broadcast the
scalar instead of materialising a parallel array: dropped the average
to 1.60x, converging to 1.12x-1.54x at 100k rows/10 columns, the
"realistic size" range. narwhals-on-polars is faster than pandas-native
throughout (0.7x-1.5x, mostly <1x at 50k+ rows). Merged into a single
narwhals path per the established decision rule - no pandas/polars
performance split - the remaining overhead is fixed per-call cost, not
scaling cost, and stays under a few ms in absolute terms even at the
largest sizes tested.
Rewrote test_rare_label_encoder.py to one parametrized test per
behaviour over @pytest.mark.parametrize("make_df", [pd.DataFrame,
pl.DataFrame]), replacing the shared pandas-only module-level fixtures
(df_enc_big, df_enc_big_na, df_enc_numeric, from tests/conftest.py,
still used by other encoder test files) with local dict constants both
backends can build from, per the CountEncoder precedent. Kept
test_when_varnames_are_numbers and the three category-dtype tests
pandas-only (integer column names and pandas Categorical dtype are
backend-specific per AGENTS.md). Split
test_max_n_categories_with_numeric_var into a pandas-only version (the
existing str()-workaround test, unchanged) plus a new polars-only
version documenting the real, expected behavioural difference: polars
can't hold mixed int/str values in one column the way pandas' object
dtype does, so a numeric variable with a string replace_with casts the
whole column to string instead of leaving frequent numeric categories
as numbers.
Verified: tests/test_encoding/test_rare_label_encoder.py - 39 passed
(up from 29, from parametrizing over both backends); full
tests/test_encoding suite - 336 passed, 17 pre-existing failures with
identical test IDs confirmed against the unmodified base_encoder.py
baseline (numpy-array-input rejection checks plus 3 MeanEncoder
inverse_transform failures from mean_encoding.py's still-unmigrated
fit() - predate this change, reproduced identically on the unmodified
rare_label.py too). flake8 clean on feature_engine and tests. mypy
clean. Module imports with pandas blocked (loaded standalone, same
technique as the base_encoder.py migration, since sibling encoder files
in this package still import pandas at module level). sphinx -W build
clean (only the pre-existing linkcode_resolve warning, confirmed
identical against the unmodified baseline). Verified every doc example
in RareLabelEncoder.rst against actual output; fixed a pre-existing,
unrelated value_counts() Series-name drift ("Name: var_A" ->
"Name: count", a pandas version difference, not caused by this
migration) while touching that page, and added a verified "With
polars" section to both the class docstring and the user guide (the
polars value_counts() example needed an explicit .sort() - unlike
pandas, its groupby-based value_counts() order isn't stable run to
run). The Titanic-dataset section of the user guide could not be
re-verified against live output in this sandboxed environment (SSL
cert verification blocks urllib by default here, though curl succeeds)
and was left untouched; a workaround (unverified SSL context) showed
matching encoder_dict_/transform output, with only an unrelated
.unique() repr-formatting difference from a newer pandas version.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* Adapt RareLabelEncoder to narwhals-returning check_X
Bind check_X / _check_transform_input_and_state results to nw_X and keep
the original native X for _check_or_select_variables, _check_na and
_check_contains_na (those helpers still expect native input, matching the
CategoricalImputer migration on narwhals-migration). Drop the redundant
nw.from_native(X) round-trips in fit() and transform(). In transform(),
detect the pandas Categorical fix-up path via nw_X.implementation
.is_pandas() and run it on a copy so the user's dataframe is not mutated.
Drop the now-unused narwhals.dependencies import.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* Use shared backend test fixtures and helpers in RareLabelEncoder tests
Replace the file-local data dicts and the _to_pandas helper - whose
polars to_pandas() call needs pyarrow, which is not a dependency, so the
polars cases failed - with the shared test structure: make_df and
data_enc_big* / data_enc_numeric fixtures, isinstance(X, make_df) plus
to_dict() checks, and pytest.raises/warns(match=re.escape(msg)).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* Use frame_to_dict after the shared helper rename in #1045
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* Group RareLabelEncoder init tests, match errors, shorten comments
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* Fix typo in RareLabelEncoder replace_with error
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* minor refactor to tests
---------
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
solegalli
added a commit
that referenced
this pull request
Sep 16, 2026
* Migrate StringSimilarityEncoder to narwhals, add polars support
fit() rebuilds encoder_dict_ with narwhals cast(nw.String)/value_counts,
matching the CountEncoder/RareLabelEncoder convention. cast() preserves
nulls as null on both pandas and polars (verified empirically), unlike
pandas' own astype(str) which stringifies NaN to "nan" - this lets
"impute" mode fill_null("") directly and "ignore" mode drop_nulls()
before casting, replacing the old "nan"/"<NA>" text-sentinel workaround
with a real null check (col.is_null()) that can't collide with a
genuine category literally named "nan" or "<NA>" (both edge cases stay
covered by test_string_dtype_with_literal_nan_strings).
transform()'s per-row difflib.SequenceMatcher similarity has no
vectorised narwhals equivalent, so it's computed once per unique value
via numpy broadcasting (np.unique's inverse index fans the small
per-unique-value matrix back out to all rows) and reassembled with
nw.new_series()/with_columns(), same pattern DecisionTreeFeatures uses
for externally-computed new columns.
Benchmarked a pandas-specific fast path (X.join(dict-of-columns), as
DecisionTreeFeatures uses) against the unified narwhals with_columns()
here across 10k-100k rows x 1-10 columns x 5-50 categories: assembly
overhead ranges 0.9x-6.25x depending on shape, but the difflib
computation itself dominates wall time by 1-3 orders of magnitude in
every realistic scenario (e.g. 30ms difflib vs <1ms assembly overhead
at 100k rows/20 categories) - even the worst synthetic case (500 output
columns) only costs ~10ms extra out of an already tens-of-ms-to-seconds
transform. Went with the unified/merged implementation: no is_pandas
split, one code path for both backends.
Rewrote tests as single parametrized cases over
@pytest.mark.parametrize("make_df", [pd.DataFrame, pl.DataFrame]),
keeping only the pandas-NA-sentinel tests (np.nan/pd.NA/None,
StringDtype) pandas-only since polars has no equivalent multi-sentinel
behavior to exercise. All doc examples (including the Titanic worked
example) re-verified against actual output; added a "With polars"
section.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* Adapt StringSimilarityEncoder to narwhals-returning check_X
Bind check_X / _check_transform_input_and_state results to nw_X and keep
the original native X for _check_or_select_variables and _check_contains_na
(those helpers still expect native input, matching the CategoricalImputer
migration on narwhals-migration). Drop the redundant nw.from_native(X)
round-trips in fit() and transform(). The empty-variables short-circuit in
transform() now returns nw_X.to_native() so callers still get a native
frame.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* Use shared backend test fixtures and helpers in StringSimilarityEncoder tests
Replace the file-local data dicts and _to_pandas/_columns helpers with the
shared test structure: make_df and data_enc* fixtures, isinstance(X, make_df)
plus to_dict() checks, and pytest.raises(match=re.escape(msg)). Tests of
pandas-specific NA sentinels and the nullable string dtype stay pandas-only.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* Use frame_to_dict after the shared helper rename in #1045
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* Check missing_values type, group StringSimilarityEncoder init tests, match errors
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* Match the fixed get_feature_names_out error message
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* refactor enc dict at the end
---------
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
solegalli
added a commit
that referenced
this pull request
Sep 18, 2026
* Migrate BaseDiscretiser to narwhals, add polars support
Shared base for ArbitraryDiscretiser, EqualFrequencyDiscretiser,
EqualWidthDiscretiser and GeometricWidthDiscretiser (not
DecisionTreeDiscretiser, which extends a different base). Only
transform() needed migrating - _fit_setup(), _get_feature_names_in()
and _check_transform_input_and_state() are inherited unchanged from
BaseNumericalTransformer, already fully narwhals-migrated.
transform()'s only pandas dependency was pd.cut, applied per column to
sort values into the bins already fixed by fit() (binner_dict_).
Replaced it with a plain numpy implementation: pandas.cut is itself
built on bins.searchsorted() internally (verified against pandas 3.0's
_bins_to_cuts source), so np.searchsorted + the same include_lowest
index-1 special case reproduces its bin-index logic exactly, with no
per-backend branch needed - values come from
nw_X.get_column(feature).to_numpy() regardless of backend, and results
are re-attached via nw.new_series()/with_columns(), so the same code
path runs for pandas and polars.
Benchmarked old pd.cut vs the new numpy+narwhals path at 10k/50k/100k
rows x 1/2/10 columns:
- return_boundaries=False (bin codes): narwhals-on-pandas lands at
~1.0-1.2x of pandas-native at realistic sizes (50k-100k rows, the
~1.9x seen only at the smallest 10k-row/1-col case is fixed
per-call overhead, sub-millisecond either way) - minimal loss,
merged into a single path, no is_pandas split. narwhals-on-polars is
~1.0-1.3x *faster* than pandas-native at every size tested.
- return_boundaries=True (interval-label strings): the numpy path is
12-20x faster than pd.cut on pandas itself (e.g. 100k rows x 10
cols: 647ms old vs 40ms new) - pd.cut's Categorical/IntervalIndex
machinery has heavy per-call overhead that np.searchsorted plus
plain string formatting avoids entirely. polars is ~1.2x faster
still than the new pandas path.
Given both branches favour or are at parity with a single numpy-driven
path, there was no case for a pandas fast-path split here.
return_boundaries=True's interval-label formatting
("(lower, upper]" text, e.g. "(-0.001, 20.0]") replicates pandas.cut's
_round_frac/_infer_precision/lowest-edge-adjustment algorithm in pure
numpy so it works identically on both backends - verified against real
pd.cut(...).astype(str) output across positive/negative/duplicate-
inducing/inf-edge bins, and against the California housing dataset
used in the existing test. return_object=True now builds a nw.Object
column (narwhals' cross-backend equivalent of pandas' "O" dtype,
already used by variable_handling for categorical-column detection)
instead of a pandas-only astype("O") call.
Verified: tests/test_discretisation full suite unchanged (109 passed,
5 pre-existing failures in test_check_estimator_discretisers.py -
sklearn's check_estimator feeds raw numpy arrays, which check_X() has
rejected since the narwhals migration's dataframe-only contract;
reproduced identically on the unmodified file). Manually diffed
transform() output against real pd.cut() across ~10 edge cases (NaN,
out-of-range values on both ends, negative bins, exact-edge values,
precision auto-widening, single bin) plus the three sibling
discretisers' documented doctest examples (EqualWidthDiscretiser,
ArbitraryDiscretiser, EqualFrequencyDiscretiser value_counts()) -
all numerically identical to old pd.cut output; the "Name: x" vs
"Name: count" and bare-fit()-repr mismatches those doctests already
show are a pre-existing pandas-3.0 doc-staleness issue unrelated to
this migration (reproduced on the unmodified files too). flake8 and
mypy clean. Module imports with pandas blocked (loaded standalone,
since sibling discretiser 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).
test_base_discretizer.py's test_transform is now parametrized over
pd.DataFrame/pl.DataFrame per AGENTS.md - its MockClassFit hard-codes
binner_dict_ rather than actually fitting, so it needed no pandas-only
logic to begin with. The other four discretisers' own test files stay
pandas-only for now: their fit() methods still call pd.cut/pd.qcut
directly and aren't migrated by this branch.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* Add shared discretiser test data fixtures
data_california, data_normal_dist, data_vartypes and data_na, shared by the
discretiser tests, as fixtures returning plain dicts built with
make_df(data). Missing values are written as None.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* Use shared backend test fixtures and helpers in BaseDiscretiser tests
Build the California housing input from the data_california fixture on the
backend under test (instead of converting a pandas frame), and check
isinstance(X, make_df) plus to_dict() contents.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* Use frame_to_dict after the shared helper rename in #1045
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
solegalli
added a commit
that referenced
this pull request
Sep 18, 2026
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
solegalli
added a commit
that referenced
this pull request
Sep 18, 2026
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
solegalli
added a commit
that referenced
this pull request
Sep 18, 2026
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
solegalli
added a commit
that referenced
this pull request
Sep 18, 2026
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
solegalli
added a commit
that referenced
this pull request
Sep 18, 2026
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
solegalli
added a commit
that referenced
this pull request
Sep 18, 2026
* Migrate ArbitraryDiscretiser to narwhals, add polars support fit() needed no changes: it already delegates entirely to the already-migrated FitFromDictMixin._fit_from_dict(). The pandas dependency was in transform()'s post-hoc NaN-introduced check, which used X[...].isnull().sum().sum() / .columns / .any() / .tolist() - pandas-only calls that broke outright on polars input coming back from the now-migrated BaseDiscretiser.transform(). Replaced it with a narwhals-based per-column check, branched on return_boundaries rather than dtype: labels (return_boundaries=True) use None for missing values, which narwhals' is_null() detects correctly on both backends. Codes (return_boundaries=False) are numeric, so a numpy float cast + np.isnan is used instead of is_null()/is_nan() directly. That numeric-cast branch isn't just style - narwhals' is_null() (and polars' own null semantics) do NOT see a boxed np.nan sitting inside a polars Object-dtype column (return_object=True's output dtype): verified with a direct repro, is_null().any() returns False on a polars Object series holding all-NaN values, silently swallowing the warning/error this method exists to raise. is_nan() isn't usable there either - narwhals raises "is_nan only supported for numeric dtype, not Object". The numpy-float-cast approach sidesteps both issues and was confirmed to raise/warn correctly across all pandas/polars x return_object x return_boundaries combinations. Benchmarked old (pandas-only) vs new (narwhals) transform() at 10k/50k/100k rows x 1/2/10 cols on pandas input: return_object=False lands at parity (0.9-1.05x, within noise); return_object=True is 1.15-1.3x slower (e.g. 100k rows x 10 cols: 36.2ms old vs 44.9ms new) since the per-variable numpy float-cast replaces one vectorized pandas isnull().sum().sum() call. This falls within the "minimal loss" band used to decide against a pandas/polars split elsewhere in this migration, so a single narwhals-driven path was kept - no is_pandas branch was added. narwhals-on-polars is faster than narwhals-on-pandas at every size tested, consistent with the base branch's own findings. Verified: tests/test_discretisation full suite (114 passed, same 5 pre-existing check_estimator failures as the unmodified base branch - reproduced there too, predates this change). Rewrote test_arbitrary_discretiser.py per AGENTS.md: one parametrized test per behavior over pd.DataFrame/pl.DataFrame (previously pandas-only), switched pytest.raises()/pytest.warns() to the match= form instead of capturing and asserting on the record. flake8 and mypy clean. Module imports with pandas blocked. sphinx -W build clean (only the pre-existing unrelated linkcode_resolve warning). Verified the existing docstring/rst examples against real output before touching: the "Name: x" vs "Name: count" and bare-fit()-repr doctest mismatches are the same pre-existing pandas-3.0 doc-staleness noted in the base branch commit (reproduced on the unmodified file too) - left alone, out of scope here. Added a "With polars" example to both the class docstring and ArbitraryDiscretiser.rst, output verified against a real run. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Use shared backend test fixtures and helpers in ArbitraryDiscretiser tests Build the California housing input from the data_california fixture on the backend under test, check isinstance(X, make_df) plus to_dict() contents, and use the make_df fixture and pytest.raises(match=re.escape(msg)). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Use frame_to_dict after the shared helper rename in #1045 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Group init tests and match errors in ArbitraryDiscretiser and BaseDiscretiser tests, check errors type Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
solegalli
added a commit
that referenced
this pull request
Sep 18, 2026
* Migrate EqualWidthDiscretiser to narwhals, add polars support
fit()'s only pandas dependency was pd.cut(bins=int, retbins=True,
duplicates="drop"), used purely to compute equal-width bin edges from
each variable's min/max (the discretised codes themselves come from
transform(), already migrated to numpy searchsorted on the prior
base_discretiser branch). Replaced it with _equal_width_edges(): a
plain numpy np.linspace(min, max, bins+1), reproducing pandas.cut's
own edge computation exactly - verified against pandas 3.0's
_nbins_to_bins/_bins_to_cuts source, including the mn==mx 0.1%-range
widening for constant columns and the duplicates="drop" collapse for
degenerate float edges. fit() now pulls all variables' values in one
nw.from_native(X).select(variables_).to_numpy() call (min/max per
column via axis=0), instead of one get_column() round-trip per
variable, following the pattern already used in CyclicalFeatures.fit().
Benchmarked old pandas-native (pd.cut per column) vs the new
narwhals+numpy fit() at 10k/50k/100k rows x 1/2/10 columns:
- narwhals-on-pandas is *faster* than the old pd.cut path everywhere
except the smallest 10k-row/1-col case (2.58x slower there, but
sub-millisecond either way - fixed per-call overhead). At realistic
sizes (50k-100k rows) it's 2-6x faster; at 100k rows x 10 cols,
19.3ms (old) vs 3.0ms (new).
- narwhals-on-polars is faster still at every size (e.g. 100k x 10:
2.9ms).
Given the new path is a speedup rather than a loss on pandas, there
was no case for a pandas fast-path split (is_pandas branch) - fit()
is a single numpy-driven code path for every backend.
Verified binner_dict_ output is numerically identical to the old
pd.cut-based fit() across 53 diff cases (random/int/negative values,
constant columns at zero/positive/negative, tiny near-duplicate float
ranges, two-point and single-value arrays, bins=1) - zero mismatches.
Also verified full fit_transform() end-to-end against the class
docstring's documented value_counts() output (pre-existing "Name: x"
vs "Name: count" pandas-3.0 staleness noted in the base branch is
unrelated to this migration) and confirmed the module fit()/transform()
round-trip works on polars with pandas import blocked at the
interpreter level.
tests/test_discretisation/test_equal_width_discretiser.py: converted
to one parametrized test per behavior over
@pytest.mark.parametrize("make_df", [pd.DataFrame, pl.DataFrame]) per
AGENTS.md, replacing the pandas-only tests. Also fixed two vacuous
assertions in the original numeric-output test (generator expressions
that were checking truthiness of an always-empty filtered sequence,
so they passed regardless of correctness) with real value comparisons
against pd.cut ground truth, and added a dedicated constant-column
case exercising the new mn==mx widening branch that pd.cut used to
handle internally.
docs/user_guide/discretisation/EqualWidthDiscretiser.rst: verified
every existing example (binner_dict_, transformed head, dtypes,
return_boundaries output) against real output - all matched, no
changes needed to those values. Fixed a pre-existing copy-paste bug
(predates this migration) where the "Return bin boundaries" code
example set up an EqualFrequencyDiscretiser instead of
EqualWidthDiscretiser. Updated the "under the hood" description that
referenced pandas.cut specifically, and added a "With polars" section
with a verified worked example.
Verified: tests/test_discretisation full suite - 116 passed, same 5
pre-existing failures as the unmodified baseline (check_estimator
feeds raw numpy arrays, rejected by check_X() since the narwhals
migration's dataframe-only contract predates this branch). flake8 and
mypy clean. sphinx -W build clean (only the pre-existing unrelated
linkcode_resolve warning, confirmed identical on the unmodified
baseline). Module imports and runs fit_transform() on polars input
with pandas blocked at the builtins.__import__ level.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* Use shared backend test fixtures and helpers in EqualWidthDiscretiser tests
Build inputs from the data_normal_dist / data_vartypes / data_na fixtures on
the backend under test instead of converting pandas frames (which needs
pyarrow for polars, so the polars cases failed), check isinstance(X, make_df)
plus to_dict() contents, and use pytest.raises(match=re.escape(msg)).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* Use frame_to_dict after the shared helper rename in #1045
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* Group init tests and match errors in EqualWidthDiscretiser tests
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* Move _equal_width_edges into EqualWidthDiscretiser as a private method
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
solegalli
added a commit
that referenced
this pull request
Sep 18, 2026
…rt (#1039) * Migrate EqualFrequencyDiscretiser.fit() to narwhals, add polars support fit()'s only pandas dependency was pd.qcut(duplicates="drop"), used to compute quantile-based bin edges per variable. Replaced it with np.quantile() on each column's narwhals-extracted numpy array, plus np.unique() to sort and drop duplicate edges - reproducing qcut's duplicates="drop" behaviour without any per-backend branch, since values come from nw_X.get_column(var).to_numpy() regardless of backend. Getting a bit-exact match (not just numerically close) took two fixes verified against pandas 3.0's pandas.core.reshape.tile.qcut source: - pandas masks out NaN before calling np.quantile(values, qs, method="linear") itself, rather than using np.nanquantile - the two are not always bit-identical. Here this distinction is moot in practice: _fit_setup() already rejects NaN in variables_, so no masking is needed - values reaching the loop are already NaN-free. - qcut nudges each quantile that isn't exactly representable in base 2 up via np.nextafter (np.linspace(0, 1, q+1) then np.putmask(quantiles, q*quantiles != np.arange(q+1), nextafter(quantiles, 1))), rounding up rather than to nearest. Skipping this shifted bin edges by ~1e-13 versus real pd.qcut output and broke an existing exact-equality test. With both applied, verified bit-exact (np.array_equal) against real pd.qcut(retbins=True) across large random floats, many-duplicate-value data, all-identical-value data, negative floats, and n<q data. Benchmarked old pd.qcut vs the new numpy+narwhals path at 10k/50k/100k rows x 1/2/10 columns: the new path is consistently faster than the old pandas-native code on BOTH backends (narwhals-on-pandas lands at 0.19x-0.47x of old pd.qcut's time, narwhals-on-polars at 0.12x-0.46x, both converging to roughly 2x faster at realistic 50k-100k row sizes). A narwhals-native quantile-expression alternative was also benchmarked (one nw.col(var).quantile(qi) expr per quantile point, batched into a single select()) - fast on polars but 2-3x *slower* than old pd.qcut on pandas, since narwhals translates each expr to a separate Series.quantile call there. Given the numpy path beats old pandas on both backends, there was no case for a pandas fast-path split. Verified: tests/test_discretisation full suite unchanged (114 passed, 5 pre-existing failures in test_check_estimator_discretisers.py, reproduced identically on the unmodified branch tip - sklearn's check_estimator feeds raw numpy arrays, rejected since the narwhals migration's dataframe-only contract). flake8 and mypy clean. Module imports with pandas blocked (loaded standalone, since sibling discretiser files in this package aren't migrated yet). sphinx -W build clean (only the pre-existing unrelated linkcode_resolve warning). test_equal_frequency_discretiser.py rewritten per AGENTS.md: each behaviour is now one test parametrized over @pytest.mark.parametrize("make_df", [pd.DataFrame, pl.DataFrame]) rather than pandas-only. docs/user_guide/discretisation/EqualFrequencyDiscretiser.rst: verified every code example against real current output. The `disc.binner_dict_` printout had two stale float digits (8099.200000000003 -> ...004, 1601.6000000000001 -> ...004, 1717.6999999999998 -> 1717.7000000000003) - reproduced identically with the OLD pd.qcut-based fit() on the same dataset/pandas version, so this predates the migration and is a doc-staleness issue, not a regression. Also corrected the "uses pandas.qcut() under the hood" line and added a "With polars" section with a verified worked example. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Use shared backend test fixtures and helpers in EqualFrequencyDiscretiser tests Build inputs from the data_normal_dist / data_vartypes / data_na fixtures on the backend under test instead of converting pandas fixtures (which needs pyarrow for polars, so the polars cases failed), check isinstance(X, make_df) plus to_dict() contents, and use pytest.raises(match=re.escape(msg)). The check that every bin code is present was vacuous and now compares the exact set of codes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Use frame_to_dict after the shared helper rename in #1045 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Group init tests and match errors in EqualFrequencyDiscretiser tests Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
solegalli
added a commit
that referenced
this pull request
Sep 18, 2026
…rt (#1041) * Migrate GeometricWidthDiscretiser.fit() to narwhals, add polars support fit()'s only pandas dependency was X[var].min()/.max() to compute the geometric progression's min/max anchors - everything downstream (the np.power/np.r_/np.sort bin-edge math) was already plain numpy and needed no changes. Replaced the pandas indexing with nw.from_native(X, eager_only=True).get_column(var).min()/.max(), which returns a numpy/python float scalar on both backends and feeds np.power identically either way. Benchmarked old pandas-native fit() vs the new narwhals-on-pandas and narwhals-on-polars paths at 10k/50k/100k rows x 1/2/10 columns (200 iterations each, min/max dominate cost either way since bin-edge math is O(bins) not O(n)): - narwhals-on-pandas: 1.0-1.3x of pandas-native at realistic sizes (50k-100k rows); the 1.8x seen only at the smallest 10k-row/1-col case is sub-millisecond fixed per-call overhead. Minimal loss - merged into a single narwhals path, no is_pandas split. - narwhals-on-polars: ~0.35-0.7x of pandas-native (i.e. 1.4-2.8x *faster*), consistent with the sibling BaseDiscretiser.transform() migration finding polars faster at every size tested. Verified: diffed new fit() bin edges against the old pandas implementation across edge cases (skewed/normal/negative-and-positive distributions, two-point range, and the min==max degenerate case) on both backends - numerically identical (exact equality, not just close). Cross-checked full fit_transform() (both return_object and return_boundaries combinations) between pandas and polars inputs - identical output values. Manually reran the GeometricWidthDiscretiser user guide's house_prices worked example (binner_dict_ and interval width numbers) against real output to confirm the docs still match current behaviour (the precision example there was already fixed in #986, prior to this branch) before adding a new "With polars" section with verified output. tests/test_discretisation/test_geometric_width_discretiser.py: the dataframe-touching tests are now parametrized over pd.DataFrame/pl.DataFrame per AGENTS.md, replacing the pandas-only df_normal_dist/df_na/df_vartypes fixtures with local dicts so the same input produces and asserts the same output on both backends (bin edges, transform values via narwhals-agnostic extraction, dtype checks, and NA-error cases). Init-only param-validation tests are unchanged since they never touch a dataframe. flake8 and mypy clean. Module imports with pandas blocked (loaded standalone, since sibling discretiser files in this package aren't migrated yet and still import pandas at their own module level). sphinx -W build clean (only the pre-existing unrelated linkcode_resolve warning). Full tests/test_discretisation suite: 114 passed, same 5 pre-existing failures as the unmodified base branch (test_check_estimator_discretisers.py - sklearn's check_estimator feeds raw numpy arrays, rejected by check_X()'s dataframe-only contract since the narwhals migration; unrelated to this change). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Use shared backend test fixtures and helpers in GeometricWidthDiscretiser tests Replace the file-local _normal_dist_data/_get_column_values/_get_column_dtype helpers with the data_normal_dist fixture, make_df, isinstance(X, make_df) plus to_dict() checks, missing values written as None, and pytest.raises(match=re.escape(msg)). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Use frame_to_dict after the shared helper rename in #1045 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Match errors and name init test in GeometricWidthDiscretiser tests Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
solegalli
added a commit
that referenced
this pull request
Sep 18, 2026
* Migrate DecisionTreeDiscretiser to narwhals, add polars support DecisionTreeDiscretiser now accepts pandas or polars input via narwhals, extending BaseNumericalTransformer directly (independent of the BaseDiscretiser migration). Never imports pandas; confirmed the module loads with pandas import blocked. Merge (single narwhals codepath, one is_pandas branch only at the final column reassembly) over split (branching at every column-selection call site): benchmarked at 10k/50k/100k rows x 1/2/10 cols, full fit+transform time is dominated by GridSearchCV tree training (10-1500ms) vs plumbing (~0.05-1.4ms per call, <1% of total even where a hand-branched pandas path was ~2x faster on the isolated plumbing microbenchmark). Merge also avoids the one-column-at-a-time write pattern that caused pandas fragmentation warnings in the DecisionTreeFeatures migration. Added optional n_jobs (default None = sequential, unchanged behaviour), parallelizing the per-variable tree fits with joblib threads, mirroring DecisionTreeFeatures. Benchmarked: net loss on small workloads (2 vars, small grid: 0.6-0.8x), real win once there's enough work (2-50 vars with a larger grid: 1.4-2.3x). Verified n_jobs=2 produces identical trees and predictions to n_jobs=None. Bug found and fixed (introduced by the base-transformer narwhals migration, not present pre-migration): check_X used to always copy its pandas input; the narwhals-based check_X no longer does, so the old transform()'s in-place `X[feature] = ...` assignments would have mutated the caller's original dataframe. Rewrote transform() to batch every replacement column and apply them in one non-mutating `.assign()` (pandas) / `.with_columns()` (polars) call instead, which also sidesteps polars' immutability and avoids per-column pandas fragmentation. Reimplemented pandas.cut's binning (bin_number/boundaries outputs) without importing pandas: np.digitize for bin assignment, and a from-scratch port of pandas' internal `_round_frac`/`_infer_precision` label-rounding algorithm (rounds each edge, bumping precision globally if that would collide two edges) so boundary labels are byte-for-byte identical to the old pd.cut output. Verified against pandas.cut directly across 500 randomized threshold/precision/value trials with zero mismatches, in addition to the existing hardcoded-value tests passing unmodified. Tests rewritten to one parametrized test per behavior over make_df in [pd.DataFrame, pl.DataFrame], replacing the pandas-only df_normal_dist/df_discretise fixtures with local data dicts (matching the DecisionTreeFeatures precedent, since those shared fixtures are still pandas-only). Fixed test_non_fitted_error, which was instantiating EqualWidthDiscretiser instead of DecisionTreeDiscretiser (a pre-existing copy-paste bug, confirmed present on main before this migration). tests/test_discretisation full suite: 123 passed (was 108 pre-migration, +15 from parametrization), same 5 pre-existing check_estimator failures (numpy-array input rejected by narwhals check_X, unrelated to this file, confirmed identical on the pre-migration baseline). flake8 and mypy clean. sphinx -W build produces only the pre-existing linkcode_resolve warning (confirmed identical on baseline). Docs: added "With polars" and "Training trees in parallel" sections, verified against real output (network available this session, so the existing fetch_openml house-prices example was re-run and confirmed still accurate). The two `binner_dict_` boundary/bin_number code blocks now display floats as plain numbers as before; current numpy's list repr actually renders them as np.float64(...), a numpy-version-only cosmetic drift present across the whole docs tree and not caused by this migration, left as-is and noted here instead. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Use shared backend test fixtures and helpers in DecisionTreeDiscretiser tests Replace the file-local _normal_dist_data/_discretise_data/_unique_sorted helpers with the shared test structure: data_normal_dist fixture, y built with make_series on the backend under test, isinstance(X, make_df) plus to_dict() checks, and pytest.raises(match=re.escape(msg)). Add a test passing the target as a list and as a numpy array, which must give the same result as a Series. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Use frame_to_dict after the shared helper rename in #1045 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Group init tests and match errors in DecisionTreeDiscretiser tests, check bin_output type Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Move DecisionTreeDiscretiser helper functions into the class as private methods Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Simplify the n_jobs note in the DecisionTreeDiscretiser user guide Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Use the scikit-learn wording for n_jobs in DecisionTreeDiscretiser Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Call is_pandas_dataframe in the condition instead of storing it Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Shorten comment in DecisionTreeDiscretiser.transform Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Reorder DecisionTreeDiscretiser tests to the test file convention Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
solegalli
added a commit
that referenced
this pull request
Sep 18, 2026
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
solegalli
added a commit
that referenced
this pull request
Sep 18, 2026
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
solegalli
added a commit
that referenced
this pull request
Sep 18, 2026
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
solegalli
added a commit
that referenced
this pull request
Sep 18, 2026
* Migrate DecisionTreeEncoder to narwhals, add polars support
Replaces the old sklearn Pipeline(OrdinalEncoder, DecisionTreeDiscretiser)
composition with a direct narwhals-based fit: each variable's categories
are ordinal-encoded via a dict built from either a target-mean group_by
(encoding_method="ordered") or plain unique-value enumeration
("arbitrary"), a decision tree is trained on the ordinal codes, and
predictions are made only on the (few) unique codes rather than the full
column, since the tree's output for a category depends only on its code -
identical result, far less prediction work for a low-cardinality variable.
The "ordered" path sorts by (mean, category) rather than mean alone,
matching the tie-break fix applied to the sibling OrdinalEncoder/
MeanEncoder migrations this session, since group_by's own row order isn't
guaranteed to match across backends for tied means.
Added n_jobs (default None, sequential, unchanged behavior), parallelizing
tree training across variables via joblib threads, following the same
pattern as DecisionTreeFeatures/DecisionTreeDiscretiser.
Verified: 56/56 own tests, full encoding suite 345 passed/17 pre-existing
failures (matches the narwhals-encoding-base baseline exactly), flake8
and mypy clean, sphinx -W build clean (only the pre-existing unrelated
linkcode_resolve warning), no pandas import in this file itself (the
package-level import chain still needs pandas only because sibling
encoders on this branch aren't migrated yet, expected given the
per-encoder parallel-branch strategy).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* Adapt DecisionTreeEncoder to narwhals-returning check_X
check_X_y now returns a narwhals frame, so bind that to nw_X and keep the
original native X for _check_or_select_variables, _check_contains_na and
_get_feature_names_in (those helpers still expect native input, matching
the CategoricalImputer migration on narwhals-migration). Drop the
redundant nw.from_native(X) in fit(); the parallel _fit_one_variable
calls reuse nw_X from check_X_y. In transform(), bind
_check_transform_input_and_state to nw_X, keep native X for
_check_contains_na, and pass nw_X to _encode (which now expects narwhals).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* Use shared backend test fixtures and helpers in DecisionTreeEncoder tests
Replace the file-local _to_backend/_assert_values helpers with the shared
test structure: make_df and data_enc* fixtures, y built with make_series on
the backend under test, isinstance(X, make_df) plus to_dict() checks, and
pytest.raises/warns(match=re.escape(msg)). Add a test passing the target as a
list and as a numpy array, which take a different code path than a Series.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* Use frame_to_dict after the shared helper rename in #1045
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* Use add_target_to_X in DecisionTreeEncoder, check encoding_method type, tidy tests
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* Shorten n_jobs docstring in DecisionTreeEncoder
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* Build DecisionTreeEncoder on OrdinalEncoder and DecisionTreeDiscretiser
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* Test DecisionTreeEncoder with integer column names
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
solegalli
added a commit
that referenced
this pull request
Sep 18, 2026
* Migrate scaling module (MeanNormalisationScaler) to narwhals, add polars support Redone from scratch off the current narwhals-migration HEAD rather than rebased forward from #979: that branch predates the dataframe_checks rewrite (#989), the variable_handling rewrite (#978), and the creation base rewrite (#990), so the delta had grown too large to carry forward safely for a module this small. fit() replaces the pandas .mean()/.max()/.min() reductions with a single narwhals+numpy path: wrap via nw.from_native, extract the variables as one batched array (nw_X.select(variables_).to_numpy()), reduce with numpy. transform()/inverse_transform() extract each variable as its own 1D array via get_column().to_numpy(), do the elementwise (x - mean) / range (or the inverse) in numpy, and write each back via nw.new_series(same_name, ...) + with_columns() -- same-named series replace the existing column in place, same as polars, rather than adding a new one the way RelativeFeatures/ MathFeatures do for their derived columns. Benchmarked narwhals-expression vs. narwhals+numpy for both fit and transform, at 100/10k/200k rows and 3/20 variables, both backends, before choosing: numpy wins by 2x-73x at small/medium scale on both pandas and polars, and even at 200k rows/polars where narwhals-expr pulls ahead it's only by ~2x, well inside the range this migration has been treating as "not worth a backend split" (CyclicalFeatures/ GeoDistanceFeatures used ~1.7x+ as the bar for splitting; nothing here gets close). One unified path, no pandas/polars branch, matching RelativeFeatures' precedent. return_empty=True guarded explicitly (mean_/range_ default to {} when variables_ is empty) -- narwhals' select([]) collapses row count too, so .to_numpy() on it would reduce over zero rows, not zero columns. Same fix CyclicalFeatures needed for the same reason. Docstring and user-guide numbers were wrong before this PR touched them, found while verifying rather than assumed: the docstring's five example values were literally the raw pre-normalization np.random.seed(42) draws, never the actual transform() output, and the user guide's inverse_transform table showed Age as a bare int (20, 21, ...) when both the pre-migration and post-migration code have always produced float64 there (multiplying by a float range always promotes the dtype, confirmed by running the pre-migration code directly). Fixed both, added a "With polars" section per AGENTS.md's doc-sync rule. Tests rewritten to the single-parametrized-over-both-backends convention (make_df=[pd.DataFrame, pl.DataFrame]) rather than kept pandas-only; all prior coverage preserved, including both class names (MeanNormalisationScaler and the deprecated MeanNormalizationScaler alias) and the deferred-attribute-assignment regression test. Verified: full test suite run twice, once against this branch and once against the unmodified narwhals-migration HEAD (via git stash) -- identical 68 pre-existing, unrelated failures in both runs (none in scaling; confirmed by diffing the two failure lists directly, not just comparing counts), 2273 -> 2287 passed (the +14 is exactly this file's new parametrized test count minus its old one). flake8 and mypy clean. * Use shared backend test fixtures and helpers in MeanNormalisationScaler tests Replace the file-local assert_df_equal/_none_to_nan helpers and parametrize decorators with the shared test structure: make_df fixture, isinstance(X, make_df) plus to_dict() checks (pytest.approx for floats), and pytest.raises(match=re.escape(msg)). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Use frame_to_dict after the shared helper rename in #1045 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Match errors, drop init asserts and rename a test in MeanNormalisationScaler tests Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Soledad Galli <solegalli@protonmail.com> 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
1 participant
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.
Standardises how the tests of narwhals-migrated transformers build pandas/polars inputs and check outputs, so every module follows one structure (modelled on the imputation tests) instead of a different set of helpers per transformer (
_to_backend,_assert_values,_cols,_to_pandas, ...).What this PR adds
tests/conftest.py: amake_dffixture parametrized overpd.DataFrame/pl.DataFrame(test idspandas/polars). Tests that request it run once per backend; pandas-only tests simply don't request it.tests/backend_helpers.py:to_dict(X)(contents as{column: values}, NaN normalised to None),null_count(X, col)andmake_series(make_df, values).tests/test_encoding/conftest.py: data shared by the encoder tests (data_enc,data_enc_rare,data_enc_na,data_enc_numeric,data_enc_big,data_enc_big_na,data_enc_top) as fixtures returning plain dicts, built withmake_df(data).Conventions applied in the PRs stacked on this one
conftest.pyas dict fixtures (missing values written asNone); data used by one file stays in that file.yis built withmake_series(exercises the Series code path), plus one list / numpy array test for transformers that takey.assert isinstance(X, make_df)and ato_dict(X)comparison (pytest.approxfor floats).pytest.raises(..., match=re.escape(msg))/pytest.warns(..., match=...).pd.testing.assert_frame_equal.Stacked on this PR (review this one first): the imputation test PR, encoders #1026-#1032, outliers #1033-#1036, discretisation #1037-#1042 and scaling #1004.
Verification
tests/suite: exactly the same failure set asnarwhals-migration(573 pre-existing failures in not-yet-migrated modules and sklearn estimator checks), nothing new.tests/test_encoding/test_count_frequency_encoder.py: 60 passed. flake8 clean.