Warn at fit when X contains likely-text columns - #1159
Open
vahidblm wants to merge 4 commits into
Open
Conversation
High-cardinality string columns that cannot be parsed as numbers are labelled TEXT by modality detection, but nothing consumed the label: they were silently swept into the OrdinalEncoder alongside real categoricals, turning near-unique text into integer noise. fit() now emits a UserWarning naming the affected columns and pointing at the fixes (numeric dtype, tabpfn-client for genuine text, or categorical_features_indices to declare a real high-cardinality category, which also silences the warning). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
The @config_context(...) instance decorating fit() wraps it via ContextDecorator.__call__, whose wrapper is a real frame in contextlib.py, so stacklevel=4 blamed contextlib instead of the caller's fit() line. Bump to 5 and pin it with warning.filename asserts in the interface tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
bejaeger
requested changes
Aug 6, 2026
| warn_if_text_features( | ||
| feature_schema, | ||
| declared_categorical_indices=self.categorical_features_indices, | ||
| ) |
Collaborator
There was a problem hiding this comment.
what about making this part of detect_feature_modalities itself?
Advantages: 1) a single place where we call this function. 2) we can add a single test to the detect_feature_modalities tests and don't need two separate tests for the classifier and regressor?
Author
There was a problem hiding this comment.
Good point! I added everything into detect_feature_modalities and simplified the tests too (so now all the tests are in test_modality_detection
Per review feedback, the likely-text warning now fires from inside detect_feature_modalities instead of being called separately by each estimator. This gives a single call site (the one place TEXT labels are produced) and a single place to test, so the classifier/regressor tests collapse into one parametrized test under the modality-detection suite. warn_if_text_features moves from validation to modality_detection; stacklevel goes 5 -> 6 for the extra detect_feature_modalities frame. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
vahidblm
force-pushed
the
warn-on-likely-text-columns
branch
from
August 7, 2026 13:57
d87662a to
0dbad7e
Compare
Refactor import statements for cleaner code.
bejaeger
approved these changes
Aug 7, 2026
bejaeger
left a comment
bejaeger
left a comment
Collaborator
There was a problem hiding this comment.
Thanks for the two changes!
Two more nits, but otherwise LGTM! approving already
| return feature_schema | ||
|
|
||
|
|
||
| def warn_if_text_features( |
| feature_schema = FeatureSchema(features=features) | ||
| # Warn here rather than at each call site: this is the single place the TEXT | ||
| # labels are produced, and they do not survive the first preprocessing step | ||
| # that rebuilds the schema. |
Collaborator
There was a problem hiding this comment.
nit: I guess this comment refers to the previous state of the code base and is not really needed
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
3 participants
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.
Issue
No GitHub issue. This was driven by repeated reports of users fitting on free-text columns, which leads to silently worse results.
Motivation
High-cardinality string columns that cannot be parsed as numbers are labelled
FeatureModality.TEXTby modality detection, but nothing used this label: they were silently swept into the sameOrdinalEncoderas real categoricals (which selects columns by dtype), turning near-unique text into near-unique integer codes, without any warnings.fit()now emits aUserWarningnaming the affected columns and pointing at the fixes: convert to a numeric dtype (numbers stored as strings), or use tabpfn-client if it's a genuine text, or pass the column incategorical_features_indicesif it's a real high-cardinality category (in this case, there won't be warnings).The check runs inside
detect_feature_modalities(the single place the TEXT labels are produced, beforeclean_datarebuilds the schema). Detection logic itself is not changed (purely diagnostic).Public API Changes
How Has This Been Tested?
tests/test_preprocessing/test_modality_detection.py: free text warns; low-cardinality strings and fully numeric strings do not warn; a numeric column with one stray "N/A" token warns; columns declared categorical (both plain string andcategorydtype) do not warn.tests/test_preprocessing/test_modality_detection.py:fitwarns and names the column,predictstays quiet, andcategorical_features_indicessilences it.stacklevel=6so the warning points at the caller'sfit()line (the@config_context(...)decorator onfitadds a contextlib wrapper frame, on top of thedetect_feature_modalitiesframe); pinned bywarning.filenameasserts in the tests.Checklist
changelog/README.md).