Use shared backend test structure in imputation tests, fix DropMissingData output type - #1046
Merged
solegalli merged 10 commits intoSep 15, 2026
Merged
Conversation
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
force-pushed
the
narwhals-imputation-test-structure
branch
from
September 15, 2026 08:06
f882f37 to
6aaade5
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
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.
Migrates the imputation tests to the shared test structure introduced in #1045 (merged), and fixes a
DropMissingDatabug that the new output-type checks caught.Tests
tests/test_imputation/conftest.py:data_na(shared by five test files) anddata_na_dob(the same data plus a never-null datetime column, forDropMissingDataandMissingIndicator), as fixtures returning plain dicts._cols,_null_count,_values,_same_values,assert_df_equal,_missing_count,_to_list,_make_series) replaced by the sharedmake_dffixture,to_dict,null_countandmake_series.isinstance(X, make_df).test_arbitrary_imputer.pynow uses the shared data, which adds theStudiescolumn, so itsn_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 bycheck_X, but dispatched onnwd.is_pandas_dataframe(X), which is never true for a narwhals frame (narwhals warns about it). As a result:missing_only=Trueon a clean training set),transform()returned the narwhals frame instead of a pandas/polars dataframe;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 inrandom_state, instead of their rounded sum or product. As a result:The
seeding_methodparameter is removed: with hashing there is nothing to add or multiply. Imputed values withseed="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 tonarwhals-migration.tests/suite: same failure set asnarwhals-migration, nothing new. flake8 and mypy clean.