Skip to content

Use shared backend test structure in imputation tests, fix DropMissingData output type - #1046

Merged
solegalli merged 10 commits into
narwhals-migrationfrom
narwhals-imputation-test-structure
Sep 15, 2026
Merged

solegalli merged 10 commits into
narwhals-migrationfrom
narwhals-imputation-test-structure

Conversation

@solegalli

@solegalli solegalli commented Sep 15, 2026 •

Copy link
Copy Markdown
Collaborator

Migrates the imputation tests to the shared test structure introduced in #1045 (merged), and fixes a DropMissingData bug that the new output-type checks caught.

Tests

  • tests/test_imputation/conftest.py: data_na (shared by five test files) and data_na_dob (the same data plus a never-null datetime column, for DropMissingData and MissingIndicator), as fixtures returning plain dicts.
  • File-local helpers (_cols, _null_count, _values, _same_values, assert_df_equal, _missing_count, _to_list, _make_series) replaced by the shared make_df fixture, to_dict, null_count and make_series.
  • Every transform output is now checked to be of the input backend with isinstance(X, make_df).
  • test_arbitrary_imputer.py now uses the shared data, which adds the Studies column, so its n_features_in_ assertions are 5.
  • RandomSampleImputer's three seed-per-observation tests are merged into one parametrized test.

Bug fix: DropMissingData
_select_rows() receives the narwhals frame returned by check_X, but dispatched on nwd.is_pandas_dataframe(X), which is never true for a narwhals frame (narwhals warns about it). As a result:

  • when no variable was selected (e.g. missing_only=True on a clean training set), transform() returned the narwhals frame instead of a pandas/polars dataframe;
  • the benchmarked pandas fast path never ran, so pandas input silently used the slower narwhals path.

It now branches on X.implementation.is_pandas() and always returns the native frame.

RandomSampleImputer: per-observation seed (#601)
With seed="observation", the seed of each observation is now a hash of the values of the variables in random_state, instead of their rounded sum or product. As a result:

  • observations with the same values in those variables receive the same imputation, regardless of their position in the dataframe (also with duplicated pandas index labels);
  • negative or very large values no longer raise an error, and different observations no longer share a seed because of rounding;
  • missing values in the seeding variables count as 0;
  • seeds are computed from the data before any variable is imputed, the same way for pandas and polars.

The seeding_method parameter is removed: with hashing there is nothing to add or multiply. Imputed values with seed="observation" differ from previous releases. The user guide is updated accordingly.

Closes #601

Verification

  • tests/test_imputation: 237 passed; the 7 failures are the pre-existing sklearn estimator checks, identical to narwhals-migration.
  • Full tests/ suite: same failure set as narwhals-migration, nothing new. flake8 and mypy clean.

solegalli and others added 3 commits September 15, 2026 10:06
Move the data duplicated across the imputer test files into
tests/test_imputation/conftest.py (data_na, and data_na_dob for the two
transformers that need a never-null datetime column), and replace the
file-local helpers (_cols, _null_count, _values, _same_values,
assert_df_equal, _missing_count, _to_list, _make_series) with the shared
ones: make_df fixture, to_dict, null_count and make_series. Every
transform output is now also checked to be of the input backend with
isinstance(X, make_df).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…s path

_select_rows() receives the narwhals frame returned by check_X, but still
dispatched with nwd.is_pandas_dataframe(X), which is never True for a
narwhals frame (narwhals warns about it). As a result:

- when no variable was selected (e.g. missing_only=True on a clean
  training set), transform() returned the narwhals frame itself instead
  of a pandas/polars dataframe;
- the benchmarked pandas fast path never ran, so pandas input silently
  went through the slower narwhals expression path.

Branch on X.implementation.is_pandas() instead, and always return the
native frame. Caught by the new isinstance(X, make_df) output checks.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The shared helper was renamed from to_dict to frame_to_dict in #1045.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@solegalli
solegalli force-pushed the narwhals-imputation-test-structure branch from f882f37 to 6aaade5 Compare September 15, 2026 08:06
@solegalli
solegalli merged commit d6e2bf9 into narwhals-migration Sep 15, 2026
4 of 10 checks passed
@solegalli
solegalli deleted the narwhals-imputation-test-structure branch September 15, 2026 09:53
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.

1 participant