Migrate DecisionTreeEncoder to narwhals, add polars support - #1026
Merged
Merged
Conversation
solegalli
force-pushed
the
narwhals-decision-tree-encoder
branch
from
August 30, 2026 22:49
94a6be6 to
48dacd6
Compare
solegalli
force-pushed
the
narwhals-decision-tree-encoder
branch
from
September 14, 2026 20:46
48dacd6 to
06752ee
Compare
Collaborator
Author
|
Updated this branch:
Locally: |
solegalli
force-pushed
the
narwhals-decision-tree-encoder
branch
from
September 15, 2026 10:06
06752ee to
dd3631f
Compare
solegalli
force-pushed
the
narwhals-decision-tree-encoder
branch
2 times, most recently
from
September 15, 2026 11:59
f374961 to
de15bcb
Compare
Collaborator
Author
|
waiting for #1042 |
solegalli
force-pushed
the
narwhals-decision-tree-encoder
branch
2 times, most recently
from
September 18, 2026 12:56
4c6f6ac to
c69c37a
Compare
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>
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>
…ests 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>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…e, tidy tests Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
solegalli
force-pushed
the
narwhals-decision-tree-encoder
branch
from
September 18, 2026 12:58
c69c37a to
10e728b
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.
Depends on #1058 (integer column names in the discretisers). Until #1058 is merged, this PR also shows its files.
Migrates
DecisionTreeEncoderto narwhals, with polars support.fituses the same pipeline as before the migration:OrdinalEncoder(encoding_method,ignore_format) followed byDecisionTreeDiscretiser(cv,scoring,param_grid,regression,random_state,n_jobs), both already migrated. The category-to-prediction mapping is read from the unique category and prediction pairs of the pipeline output, rounded withprecision.n_jobsparameter, passed toDecisionTreeDiscretiserto fit the trees in parallel.encoder_dict_is identical to the previous hand-written implementation in 37 cases (pandas and polars, both encoding methods, regression and classification,precisionNone/0/2, list target, numeric variables,n_jobs=2, integer column names). The pipeline is about 10% slower on pandas and 2% slower on polars than that implementation, because it transforms and predicts every row; most of the time is the grid search in both.