Skip to content

refactor creation base for narwhals compatibility - #990

Merged
solegalli merged 4 commits into
narwhals-migrationfrom
narwhals-creation-base
Aug 24, 2026
Merged

refactor creation base for narwhals compatibility#990
solegalli merged 4 commits into
narwhals-migrationfrom
narwhals-creation-base

Conversation

@solegalli

Copy link
Copy Markdown
Collaborator

No description provided.

…andas imports

BaseCreation, BaseNumericalTransformer, and mixins.py (TransformXyMixin,
FitFromDictMixin, GetFeatureNamesOutMixin) are used by every transformer in
the creation module, so their remaining pandas-only code blocked a
polars-only install regardless of which transformer was migrated. Adds
pandas fast paths (benchmarked ~2-11x) alongside narwhals-generic branches,
replaces y.loc[X.index] row alignment in TransformXyMixin with a
narwhals with_row_index()-based mechanism for non-pandas backends, and adds
test_base_creation.py plus polars coverage for transform_x_y.
@solegalli
solegalli changed the base branch from main to narwhals-migration August 24, 2026 13:08
Comment thread feature_engine/_base_transformers/mixins.py
Comment thread feature_engine/_base_transformers/mixins.py Outdated
…process rules to AGENTS.md

The 48 failures here were pre-existing (unrelated to the to_list() fix,
confirmed identical before/after): check_X no longer accepts raw numpy
arrays, and most of this file's tests fit() on df_vartypes.to_numpy() or
feed a raw-array-outputting sklearn transformer upstream. Fixes:
- array-input tests converted to set feature_names_in_/n_features_in_
  directly, since that's the only way left to reach the mixin's x0/x1/...
  naming branch (fit() rejects arrays outright now).
- SimpleImputer/PolynomialFeatures steps get .set_output(transform="pandas")
  so they hand a dataframe to the next pipeline step instead of an array -
  this is also the fix any real user chaining sklearn + feature-engine
  transformers in a Pipeline now needs.
- pure Mock-only tests (no sklearn transformer involved) parametrized over
  pandas and polars.
Also adds two AGENTS.md rules: run a changed function/class's tests and
resolve any failures, and keep user-guide docs in sync with new
transformer functionality.
This branch handled feature_names_in_ == ["x0", "x1", ...], the naming
sklearn gives an estimator fit on a raw array. check_X no longer accepts
arrays (dataframe-only input, per AGENTS.md), so fit() can never produce
that pattern anymore - the branch, its indices=True path in
_remove_feature_names, and get_support(indices=True) were all unreachable.
It was also a latent correctness gap: a dataframe with columns genuinely
named x0..xn would have hit this branch and skipped the usual
input_features-must-match-feature_names_in_ validation.

Verified via git history (#519, 2022) this was built for the old
array-accepting check_X; confirmed no other code in the library still
generates x0/x1/... names. Removed the branch, its now-single-path
_remove_feature_names, and the tests that existed only to reach it -
replaced by tests/test_base_transformers/test_get_feature_names_out_mixin.py's
remaining pandas+polars dataframe coverage, which already exercises the
same validation/renaming logic through the one reachable path.
@solegalli
solegalli merged commit 45b9d1a into narwhals-migration Aug 24, 2026
3 of 10 checks passed
@solegalli
solegalli deleted the narwhals-creation-base branch August 24, 2026 15:00
solegalli added a commit that referenced this pull request Aug 30, 2026
* Migrate creation/mixins shared base classes to narwhals, remove all pandas imports

BaseCreation, BaseNumericalTransformer, and mixins.py (TransformXyMixin,
FitFromDictMixin, GetFeatureNamesOutMixin) are used by every transformer in
the creation module, so their remaining pandas-only code blocked a
polars-only install regardless of which transformer was migrated. Adds
pandas fast paths (benchmarked ~2-11x) alongside narwhals-generic branches,
replaces y.loc[X.index] row alignment in TransformXyMixin with a
narwhals with_row_index()-based mechanism for non-pandas backends, and adds
test_base_creation.py plus polars coverage for transform_x_y.

* Apply suggestion from @FBruzzesi

* Fix test_get_feature_names_out_mixin.py after to_list() removal, add process rules to AGENTS.md

The 48 failures here were pre-existing (unrelated to the to_list() fix,
confirmed identical before/after): check_X no longer accepts raw numpy
arrays, and most of this file's tests fit() on df_vartypes.to_numpy() or
feed a raw-array-outputting sklearn transformer upstream. Fixes:
- array-input tests converted to set feature_names_in_/n_features_in_
  directly, since that's the only way left to reach the mixin's x0/x1/...
  naming branch (fit() rejects arrays outright now).
- SimpleImputer/PolynomialFeatures steps get .set_output(transform="pandas")
  so they hand a dataframe to the next pipeline step instead of an array -
  this is also the fix any real user chaining sklearn + feature-engine
  transformers in a Pipeline now needs.
- pure Mock-only tests (no sklearn transformer involved) parametrized over
  pandas and polars.
Also adds two AGENTS.md rules: run a changed function/class's tests and
resolve any failures, and keep user-guide docs in sync with new
transformer functionality.

* Remove dead array-input branch from GetFeatureNamesOutMixin

This branch handled feature_names_in_ == ["x0", "x1", ...], the naming
sklearn gives an estimator fit on a raw array. check_X no longer accepts
arrays (dataframe-only input, per AGENTS.md), so fit() can never produce
that pattern anymore - the branch, its indices=True path in
_remove_feature_names, and get_support(indices=True) were all unreachable.
It was also a latent correctness gap: a dataframe with columns genuinely
named x0..xn would have hit this branch and skipped the usual
input_features-must-match-feature_names_in_ validation.

Verified via git history (#519, 2022) this was built for the old
array-accepting check_X; confirmed no other code in the library still
generates x0/x1/... names. Removed the branch, its now-single-path
_remove_feature_names, and the tests that existed only to reach it -
replaced by tests/test_base_transformers/test_get_feature_names_out_mixin.py's
remaining pandas+polars dataframe coverage, which already exercises the
same validation/renaming logic through the one reachable path.
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.

2 participants