Skip to content

Fix GapEncoder(init="k-means") crash on columns containing nulls - #2238

Open
Hrafz wants to merge 1 commit into
skrub-data:mainfrom
Hrafz:fix/gap-encoder-kmeans-init-null-crash
Open

Fix GapEncoder(init="k-means") crash on columns containing nulls#2238
Hrafz wants to merge 1 commit into
skrub-data:mainfrom
Hrafz:fix/gap-encoder-kmeans-init-null-crash

Conversation

@Hrafz

@Hrafz Hrafz commented Aug 1, 2026

Copy link
Copy Markdown

Addresses issue
#2237

Description

GapEncoder(init="k-means") crashes on any column containing a missing
value, while the two other init options fit the same column fine:

import pandas as pd
from skrub import GapEncoder

s = pd.Series(["alice", "bob", None, "alice", pd.NA, float("nan")], name="s")

GapEncoder(n_components=3, init="k-means++").fit_transform(s)  # OK
GapEncoder(n_components=3, init="random").fit_transform(s)     # OK
GapEncoder(n_components=3, init="k-means").fit_transform(s)
# AttributeError: 'NAType' object has no attribute 'lower'

Expected: init="k-means" handles the column's nulls the same way the other
two init options do (nulls are treated as the empty string throughout
GapEncoder, see skrub._utils.unique_strings and the existing
test_missing_values).

Actual: it raises from scikit-learn's text preprocessor. The exception type
depends on the column's dtype rather than on which sentinel appears first:

column init="k-means" raises
object dtype containing np.nan ValueError: np.nan is an invalid document, expected byte or unicode string.
object dtype containing None ValueError: np.nan is an invalid document, expected byte or unicode string.
string dtype containing pd.NA AttributeError: 'NAType' object has no attribute 'lower'

init="k-means++" and init="random" fit all three without error.

Cause: _init_vars builds the null-substituted array unq_X via
unique_strings(X, is_null) and uses it everywhere else in the method, but
was calling self._init_w(unq_V[lookup], X) with the raw X (still
containing the original null). Only the "k-means" branch touches X
directly — it re-vectorizes it from scratch inside get_kmeans_prototypes,
which is where the null reaches the preprocessor. "k-means++" and
"random" only ever consume the already-vectorized V.

Fix: pass unq_X[lookup] instead of X — the same null-substituted,
per-sample reconstruction already used throughout the rest of the method.
When there are no nulls, unq_X[lookup] == X, so this is a no-op for the
common case.

Checklist

  • I have read the contributing guidelines
  • I have added tests that verify the bug fix
  • I have added an entry to CHANGES.rst describing the fix
  • My code follows the code style of this project
  • I have checked my code and corrected any misspellings

How Has This Been Tested?

test_missing_values is now parametrized over init, so it also exercises
"k-means" (previously only the default "k-means++").

Before the fix (test change only, source untouched):

skrub/tests/test_gap_encoder.py::test_missing_values[pandas-numpy-dtypes-k-means] FAILED
skrub/tests/test_gap_encoder.py::test_missing_values[pandas-nullable-dtypes-k-means] FAILED
2 failed, 4 passed, 44 deselected

After the fix:

skrub/tests/test_gap_encoder.py -q
50 passed

The full skrub/tests suite passes with no failures and no change in the
skipped/xfailed/xpassed counts beyond the newly-passing parametrizations.

GapEncoder(init="k-means") crashed on any column containing a missing
value, while init="k-means++" and init="random" fit the same column
without error. _init_vars builds the null-substituted array unq_X via
unique_strings(X, is_null) and uses it everywhere else in the method,
but was calling self._init_w(unq_V[lookup], X) with the raw X (still
containing None/NaN). init="k-means" is the only branch that touches X
directly: it re-vectorizes it from scratch inside get_kmeans_prototypes,
which is where the null reaches scikit-learn's text preprocessor and
crashes. "k-means++" and "random" only ever consume the already-
vectorized V, so they are unaffected. The exact exception depends on
which null sentinel is hit first (AttributeError for None/pd.NA,
ValueError for np.nan).

Fix: pass unq_X[lookup] instead of X, the same per-sample reconstruction
already used throughout the method. When there are no nulls,
unq_X[lookup] == X, so this is a no-op for the common case.

test_missing_values is now parametrized over init, so it also exercises
"k-means" (previously only the default "k-means++"). Before the fix:
2/6 parametrizations fail with the errors above; after: all 6 pass.
@Hrafz
Hrafz force-pushed the fix/gap-encoder-kmeans-init-null-crash branch from 4afe1a8 to a994e9c Compare August 1, 2026 18:10
@rcap107

rcap107 commented Aug 18, 2026

Copy link
Copy Markdown
Member

Hi @Hrafz, thanks for the PR. We'll be looking at this and #2240 in the next few days (most maintainers were off in the past few weeks)

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