Migrate EqualWidthDiscretiser to narwhals, add polars support - #1040
Merged
Merged
Conversation
solegalli
force-pushed
the
narwhals-equal-width-discretiser
branch
from
September 14, 2026 20:46
c6f4877 to
5143c64
Compare
Collaborator
Author
|
Updated this branch:
Locally: |
solegalli
force-pushed
the
narwhals-equal-width-discretiser
branch
from
September 15, 2026 10:06
5143c64 to
89e7c7e
Compare
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>
… 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>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
solegalli
force-pushed
the
narwhals-equal-width-discretiser
branch
from
September 18, 2026 11:17
89e7c7e to
31244fc
Compare
Co-Authored-By: Claude Opus 5 <noreply@anthropic.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.
Migrates
EqualWidthDiscretiserto narwhals with polars support.fit()'s only pandas dependency waspd.cut(bins=int, retbins=True, duplicates="drop"), used purely to compute equal-width bin edges from each variable's min/max (the discretised codes come fromtransform(), migrated onnarwhals-discretisation-base). Replaced with_equal_width_edges(): a plainnp.linspace(min, max, bins+1), reproducingpandas.cut's own edge computation exactly — verified against pandas 3.0's_nbins_to_bins/_bins_to_cutssource, including themn == mx0.1%-range widening for constant columns and theduplicates="drop"collapse for degenerate float edges.fit()now pulls all variables in one.select(variables_).to_numpy()call (min/max per column viaaxis=0), followingCyclicalFeatures.fit().Merge vs split: benchmarked old pandas-native (
pd.cutper column) vs the new narwhals+numpyfit()at 10k/50k/100k rows × 1/2/10 cols. narwhals-on-pandas is faster everywhere except the smallest 10k/1-col case (2.58x slower, sub-ms); at realistic sizes 2–6x faster (100k×10: 19.3ms → 3.0ms). polars faster still. The new path is a speedup on pandas — nois_pandasbranch, single numpy-driven path for every backend.Verified
binner_dict_numerically identical to the oldpd.cut-basedfit()across 53 diff cases (random/int/negative values, constant columns, tiny near-duplicate float ranges, two-point and single-value arrays, bins=1) — zero mismatches.Tests:
test_equal_width_discretiser.pyconverted to one parametrized test per behaviour over[pd.DataFrame, pl.DataFrame]. Also fixed two vacuous assertions in the original numeric-output test (generator expressions checking truthiness of an always-empty sequence) with real comparisons againstpd.cutground truth, and added a constant-column case exercising themn == mxwidening branch. Fixed a pre-existing copy-paste bug inEqualWidthDiscretiser.rst(a "Return bin boundaries" example set up anEqualFrequencyDiscretiser).Verified:
tests/test_discretisation— 116 passed, same 5 pre-existingcheck_estimatorfailures. flake8 / mypy clean, sphinx -W clean. Runsfit_transform()on polars with pandas blocked atbuiltins.__import__.Stacked on
narwhals-discretisation-base(its own PR). Until that merges this PR's diff also contains the sharedBaseDiscretisercommit; review that one first.