From 0ad8092577c195cb38d1aa68f06cc072207bf9b8 Mon Sep 17 00:00:00 2001
From: ChrisW09 <50968720+ChrisW09@users.noreply.github.com>
Date: Mon, 27 Jul 2026 22:43:09 +0200
Subject: [PATCH] fix(preprocessor): reject an unrecognized scaling value

``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>
---
 pretab/pipeline/numerical.py | 11 ++++++++-
 tests/test_method_aliases.py | 43 ++++++++++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+), 1 deletion(-)

diff --git a/pretab/pipeline/numerical.py b/pretab/pipeline/numerical.py
index 56bd7aa..dfb0e6b 100644
--- a/pretab/pipeline/numerical.py
+++ b/pretab/pipeline/numerical.py
@@ -128,9 +128,18 @@ def get_numerical_transformer_steps(
         "minmax": ("minmax", MinMaxScaler(feature_range=(-1, 1))),
     }
 
-    # Add optional scaling step only if not already part of method
+    # Add optional scaling step only if not already part of method. An
+    # unrecognized name used to fall through the membership test below and
+    # silently produce an unscaled pipeline, so a typo disabled scaling instead
+    # of reporting it -- unlike ``method``, which is validated a few lines down.
     if scaling is not None:
         scaling = resolve_method(scaling, NUMERICAL_METHODS, NUMERICAL_ALIASES)
+        if scaling not in scalers and scaling != "none":
+            raise invalid_param_error(
+                "get_numerical_transformer_steps", "scaling", scaling,
+                "must name a scaler, or be None / 'none' to disable scaling",
+                valid={*scalers, "none"},
+            )
     if scaling in scalers and scaling != method:
         steps.append(scalers[scaling])
 
diff --git a/tests/test_method_aliases.py b/tests/test_method_aliases.py
index 4f5e03f..cd4962e 100644
--- a/tests/test_method_aliases.py
+++ b/tests/test_method_aliases.py
@@ -3,6 +3,7 @@
 import pytest
 
 from pretab.core.exceptions import InvalidParamError
+from pretab.pipeline import get_numerical_transformer_steps
 from pretab.pipeline.registry import (
     CATEGORICAL_ALIASES,
     CATEGORICAL_METHODS,
@@ -144,3 +145,45 @@ def test_unknown_method_still_raises(sample_data):
     X, y = sample_data
     with pytest.raises(InvalidParamError):
         Preprocessor(numerical_method="definitely-not-a-method").fit_transform(X, y)
+
+
+# --------------------------------------------------------------------------- #
+# An unrecognized ``scaling`` value must raise, not silently disable scaling.
+#
+# ``resolve_method`` returns the input lowercased when it recognizes nothing, so
+# the membership test just failed and the pipeline came out unscaled -- unlike
+# ``method``, which is validated explicitly.
+# --------------------------------------------------------------------------- #
+@pytest.mark.parametrize("scaling", ["not_a_scaler", "minmaxx", "standardisation"])
+def test_unknown_scaling_raises(scaling):
+    with pytest.raises(InvalidParamError, match="scaling"):
+        get_numerical_transformer_steps("ple", scaling=scaling)
+
+
+@pytest.mark.parametrize(
+    ("scaling", "expected"),
+    [
+        ("minmax", "minmax"),
+        ("MinMax", "minmax"),
+        ("min-max", "minmax"),
+        ("zscore", "scaler"),
+        ("standardization", "scaler"),
+    ],
+)
+def test_known_scaling_still_adds_its_step(scaling, expected):
+    steps = [name for name, _ in get_numerical_transformer_steps("ple", scaling=scaling)]
+    assert expected in steps
+
+
+@pytest.mark.parametrize("scaling", [None, "none", "passthrough"])
+def test_scaling_can_still_be_disabled(scaling):
+    steps = [name for name, _ in get_numerical_transformer_steps("ple", scaling=scaling)]
+    assert steps == ["imputer", "ple"]
+
+
+def test_unknown_scaling_raises_through_the_preprocessor():
+    rng = np.random.default_rng(0)
+    frame = pd.DataFrame({"a": rng.normal(size=50)})
+
+    with pytest.raises(InvalidParamError, match="scaling"):
+        Preprocessor(scaling="minmaxx").fit(frame, rng.normal(size=50))