Skip to content

fix(binning): make get_feature_names_out follow the sklearn contract - #42

Open
ChrisW09 wants to merge 1 commit into
mainfrom
fix/custombin-feature-names-contract
Open

fix(binning): make get_feature_names_out follow the sklearn contract#42
ChrisW09 wants to merge 1 commit into
mainfrom
fix/custombin-feature-names-contract

Conversation

@ChrisW09

Copy link
Copy Markdown
Collaborator

Fixes #36.

Problem

CustomBinTransformer.get_feature_names_out() raised when called with no arguments and
returned a plain list when given them. Both break scikit-learn's contract, and the first
breaks Pipeline.get_feature_names_out() for any pipeline containing the transformer:

t = CustomBinTransformer(output_dim=4).fit(X)
t.get_feature_names_out()
# InvalidParamError: input_features must be specified

Pipeline([("bin", CustomBinTransformer(output_dim=4))]).fit(X).get_feature_names_out()
# InvalidParamError: input_features must be specified

The package had already fixed exactly this for three sibling transformers — the docstring of
tests/test_feature_names_out.py records it as "get_feature_names_out(None) defaults to
generated x0, x1, ... names instead of raising, for NoTransformer, ToFloatTransformer
and ContinuousOrdinalTransformer"
— but CustomBinTransformer was missed.

Fix

Mirror NoTransformer.get_feature_names_out: generate x0, x1, ... when input_features is
None, always return an ndarray, and guard on being fitted (which the old version did not do
at all).

no args    -> ['x0'] | ndarray
with names -> ['f']  | ndarray
Pipeline   -> ['x0']

Changed existing expectations

Both flagged in the issue:

  • test_custom_bin_transformer_feature_names_out asserted names == ["feature1"], which
    only holds for a list. Now checks the ndarray type and its contents.
  • test_custom_bin_transformer_feature_names_out_raises asserted the no-argument call
    raises — i.e. it pinned the defect. Replaced with
    test_custom_bin_transformer_feature_names_out_before_fit_raises, checking the
    NotFittedError that the new check_is_fitted guard produces, which is the assertion that
    should have been there.

Tests

Three added beyond those: generated default names, the pipeline call, and
len(get_feature_names_out()) == transform(X).shape[1].

Full suite: 483 passed, 9 xfailed. ruff check pretab clean apart from the pre-existing
RUF100; the remaining I001/B017/RUF043 in this test file all predate the change.
pyright unchanged at 71.

🤖 Generated with Claude Code

``CustomBinTransformer.get_feature_names_out()`` raised without arguments and
returned a plain list with them, so it broke ``Pipeline.get_feature_names_out()``
for any pipeline containing it:

    t.get_feature_names_out()   -> InvalidParamError: input_features must be specified
    Pipeline([...]).get_feature_names_out() -> same

Generate ``x0, x1, ...`` when ``input_features`` is None, return an ndarray, and
guard on being fitted -- matching ``NoTransformer``, ``ToFloatTransformer`` and
``ContinuousOrdinalTransformer``, which already had this treatment (see the
docstring of tests/test_feature_names_out.py).

Two existing tests pinned the old behaviour and are updated:
``test_custom_bin_transformer_feature_names_out`` compared against a list, and
``..._out_raises`` asserted the no-argument call raises. The latter becomes a
``NotFittedError`` check for the un-fitted call, which is the guard that should
actually be there.

Closes #36

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@ChrisW09
ChrisW09 force-pushed the fix/custombin-feature-names-contract branch from 4d2b9ee to 07e56cc Compare July 27, 2026 20:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(binning): make get_feature_names_out follow the sklearn contract

1 participant