fix(binning): learn bin edges at fit instead of per transform call - #30
Open
ChrisW09 wants to merge 1 commit into
Open
fix(binning): learn bin edges at fit instead of per transform call#30ChrisW09 wants to merge 1 commit into
ChrisW09 wants to merge 1 commit into
Conversation
``CustomBinTransformer.fit`` recorded only ``n_features_in_``; ``transform``
called ``pd.cut(..., retbins=True)`` on whatever batch it was handed and used
those edges. The encoding therefore depended on the batch, not on the training
data:
fit(linspace(0, 10, 100))
transform([0, 2.5, 5, 7.5, 10]) -> [0 0 1 2 3]
transform([0, 2.5, 5, 7.5, 10, 100]) -> [0 0 0 0 0 3]
Identical rows, different codes, and neither result reflected the 100-row fit.
Silent train/serve skew: no error, and the codes look plausible.
Compute the edges in ``fit``, store them as ``bin_edges_``, and reuse them
verbatim. Values outside the fitted range are clipped into the end bins so
``transform`` never emits NaN into an integer column (``pd.cut`` returns NaN
outside its edges).
The numeric-dtype guard moves to ``fit`` alongside the edge computation, so
string input is now rejected at fit rather than at first transform.
Also removes the ``X.shape[0] <= 2`` guard from ``transform``, which made
single-row inference impossible. It was only meaningful for deriving an
equal-width range -- something that now happens at fit, where pandas handles
small inputs by widening the range.
Closes #11
Co-Authored-By: Claude Opus 5 (1M context) <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
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.
Fixes #11.
Problem
fitrecorded onlyn_features_in_andtotal_output_dim_.transformcalledpd.cut(X.squeeze(), bins=bins_spec, retbins=True)on whatever batch it was handed and usedthose edges — so the encoding depended on the transform batch, not on the training data:
Same five rows, different codes — and neither result reflects the 100-row fit. This is silent
train/serve skew: no error, and the codes look entirely plausible.
transformalso rejected fewer than three rows, so single-row inference was impossible.Fix
fit, store asbin_edges_(one array per column), reuse verbatim.pd.cutreturnsNaNoutside its edges, whichwould put
NaNinto an otherwise integer column; clipping puts unseen extremes in thefirst/last bin instead. Flagged as an open question in the issue — this is the choice I
made, easy to change to a reserved code if you'd rather.
fit, alongside the edge computation, so string input isrejected at fit rather than at first transform.
X.shape[0] <= 2guard fromtransform. It only ever mattered for deriving anequal-width range, which now happens at fit — where pandas handles small inputs by
widening the range.
Result
One existing test changed
test_custom_bin_transformer_raises_on_invalid_shapeasserted thattransform(np.array([[0.1]]))raises"Input must have more than 2 observations."— i.e. itencoded the behaviour this issue asks to remove. I repurposed it to cover the real shape
guard (a 2-column input raises
PretabDataError) and addedtest_custom_bin_transformer_raises_before_fitfor theNotFittedErrorpath that theun-fitted call now takes.
No other existing test needed touching. Two tests that
fiton a single row still pass,since pandas widens the range for degenerate input.
Tests
Seven added: edges learned at fit, codes independent of batch composition, codes reflecting
the fitted range, single-row transform, out-of-range clipping, explicit-edge round-trip, and
string input rejected at fit.
Full suite: 488 passed, 9 xfailed. pyright unchanged at 71. Ruff on the touched test file
goes from 4 pre-existing errors to 3 — the
RUF043disappears because the test I rewrote nowuses a raw-string
match=.🤖 Generated with Claude Code