Migrate StringSimilarityEncoder to narwhals, add polars support - #1031
Open
solegalli wants to merge 2 commits into
Open
Migrate StringSimilarityEncoder to narwhals, add polars support#1031solegalli wants to merge 2 commits into
solegalli wants to merge 2 commits into
Conversation
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>
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>
solegalli
force-pushed
the
narwhals-similarity-encoder
branch
from
August 30, 2026 22:44
6281fcb to
f263a18
Compare
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
StringSimilarityEncoderto narwhals with polars support.fit()rebuildsencoder_dict_with narwhalscast(nw.String)/value_counts, matching theCountEncoder/RareLabelEncoderconvention.cast()preserves nulls as null on both backends (verified), unlike pandas'astype(str)which stringifies NaN to"nan"— so"impute"mode canfill_null("")directly and"ignore"modedrop_nulls()before casting. This replaces the old"nan"/"<NA>"text-sentinel workaround with a realcol.is_null()check that can't collide with a genuine category named"nan"or"<NA>"(edge cases still covered bytest_string_dtype_with_literal_nan_strings).transform()'s per-rowdifflib.SequenceMatchersimilarity has no vectorized narwhals equivalent, so it's computed once per unique value via numpy broadcasting (np.unique's inverse index fans the small per-unique matrix back to all rows) and reassembled withnw.new_series()/with_columns()— same patternDecisionTreeFeaturesuses for externally-computed columns.Merge vs split: benchmarked a pandas-specific fast path (
X.join(dict-of-columns)) vs the unifiedwith_columns()across 10k–100k rows × 1–10 cols × 5–50 categories. Assembly overhead ranges 0.9x–6.25x by shape, but thedifflibcomputation dominates wall time by 1–3 orders of magnitude in every realistic case (e.g. 30ms difflib vs <1ms assembly at 100k rows/20 categories). Unified implementation, nois_pandassplit.Tests rewritten as single parametrized cases over
make_df in [pd.DataFrame, pl.DataFrame]; pandas-NA-sentinel tests (np.nan/pd.NA/None,StringDtype) kept pandas-only. All doc examples including the Titanic worked example re-verified against actual output; "With polars" section added.Stacked on #999 (
narwhals-encoding-base). Until that merges this PR's diff also contains the sharedCategoricalMethodsMixincommit; review #999 first.