fix(preprocessor): reject an unrecognized scaling value - #41
Open
ChrisW09 wants to merge 1 commit into
Open
Conversation
``resolve_method`` returns its input lowercased when it recognizes nothing, so
an unknown ``scaling`` fell straight through the ``scaling in scalers``
membership test and produced a pipeline with no scaler at all:
scaling='minmaxx' -> ['imputer', 'ple'] silently unscaled
scaling='not_a_scaler' -> ['imputer', 'ple'] silently unscaled
A typo therefore disabled scaling instead of reporting it, while the same typo
in ``method`` raises a few lines further down.
Validate after alias resolution, keeping ``None`` and ``"none"`` as the
documented ways to disable scaling.
This is worth guarding because the failure is invisible: the fit succeeds, the
output widths are unchanged, and only the numeric scale differs -- so the
mistake shows up, if at all, as a quietly worse model.
Closes #35
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Fixes #35.
Problem
resolve_methodreturns its input lowercased when it recognizes nothing, so an unrecognizedscalingvalue fell through the membership test and produced a pipeline with no scaler:Preprocessor(scaling="minmaxx").fit(df, y)completed without complaint. Meanwhile the sametypo in
numerical_methodraises, a few lines further down in the same function.Fix
Validate after alias resolution, keeping
Noneand"none"as the documented ways to turnscaling off:
Alias resolution is untouched, so
"MinMax","min-max","zscore"and"standardization"all keep working.Why it matters more than a typical typo guard
The failure is invisible. The fit succeeds, the output widths are identical, and only the
numeric scale of the features differs — so the mistake surfaces, if at all, as a quietly
worse model rather than an error.
Tests
Nine added to
tests/test_method_aliases.py: three unknown values that must raise, fiveknown spellings (including aliases) that must still insert their step, three disabling
spellings that must remain no-ops, and one end-to-end check through the
Preprocessor.Full suite: 492 passed, 9 xfailed.
ruff checkclean on changed files; pyright unchanged atits pre-existing 71 errors.
🤖 Generated with Claude Code