Repository navigation
Conversation
scikit-learn only keeps feature names when every column name is exactly of type str, so StrEnum (and other str subclass) column names were dropped: the search used x0, x1, and predict on the same DataFrame raised "Input X contains NaN", because it reindexes the columns by x0, x1. Convert such column names to plain strings in fit. Fixes astroautomata#887 Assisted-by: Claude:claude-opus-5-5
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0916a9877a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if any(isinstance(c, str) and type(c) is not str for c in X.columns): | ||
| X = X.copy(deep=False) | ||
| X.columns = [ | ||
| str.__str__(c) if isinstance(c, str) else c for c in X.columns |
There was a problem hiding this comment.
When a DataFrame combines a str subclass with any non-string label (for example [Cols.A, 0]) on scikit-learn 1.2+, this conversion turns only Cols.A into an exact str. Scikit-learn then sees mixed str/int feature-name types and rejects fit, whereas the parent code saw no exact str type and accepted the data by falling back to generated feature names. Restrict this normalization to columns that are all string-like, or normalize non-string labels consistently, so existing mixed-label training does not fail.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch, thanks. With columns like [Cols.A, 0], converting only Cols.A gave scikit-learn a mix of str and int names, and fit raised a TypeError. On master the same frame is accepted and the names are ignored (x0, x1). a9d1218 converts only when every column name is a string, so mixed names keep the current behaviour, and it adds a test for that case.
Converting the str subclass names next to a non-string name, as in [Cols.A, 0], gave scikit-learn a mix of str and int names, and it raised a TypeError. Such a DataFrame was accepted before, with the names ignored and x0, x1 used instead. Convert only when every column name is a string, and test the mixed case. Assisted-by: Claude:claude-opus-5-5
Summary
Fixes #887
scikit-learn only keeps feature names when every column name is exactly of type
str, so DataFrame columns named byStrEnummembers (or any otherstrsubclass) lost their names: equations usedx0,x1, andpredicton the same DataFrame raisedInput X contains NaN, because it reindexes the columns byfeature_names_in_.variable_namesgave no way around it, since it is reset whenXis a DataFrame.fitnow converts such column names to plain strings, next to the existing handling of spaces. The caller's DataFrame is not changed.Tests
test_str_subclass_dataframe_columnsfits and predicts with column names from astrmixinEnum(StrEnumneeds Python 3.11, and CI also runs 3.10; scikit-learn ignores both the same way). It fails without the change.Assisted-by: Claude:claude-opus-5-5