From fb463d8f40962ee4b22bd49cc292cd45c46224ae Mon Sep 17 00:00:00 2001 From: Soledad Galli Date: Tue, 15 Sep 2026 13:30:54 +0200 Subject: [PATCH 1/2] Add add_target_to_X helper, check init param types in encoders Co-Authored-By: Claude Opus 5 --- feature_engine/encoding/_helper_functions.py | 20 +++++- feature_engine/encoding/base_encoder.py | 5 +- feature_engine/encoding/count_frequency.py | 5 +- feature_engine/encoding/mean_encoding.py | 26 ++++---- .../test_categorical_init_mixin.py | 7 ++- .../test_categorical_init_mixin_na.py | 16 ++--- .../test_count_frequency_encoder.py | 22 +++++-- tests/test_encoding/test_helper_functions.py | 53 ++++++++++++---- tests/test_encoding/test_mean_encoder.py | 63 +++++++++++++------ 9 files changed, 152 insertions(+), 65 deletions(-) diff --git a/feature_engine/encoding/_helper_functions.py b/feature_engine/encoding/_helper_functions.py index 521f6e31e..d9d047311 100644 --- a/feature_engine/encoding/_helper_functions.py +++ b/feature_engine/encoding/_helper_functions.py @@ -1,3 +1,9 @@ +import narwhals as nw +import narwhals.dependencies as nwd + +TARGET_NAME = "__feature_engine_target__" + + def check_parameter_unseen(unseen, accepted_values): if not isinstance(accepted_values, list) or not all( isinstance(item, str) for item in accepted_values @@ -6,8 +12,20 @@ def check_parameter_unseen(unseen, accepted_values): "accepted_values should be a list of strings. " f" Got {accepted_values} instead." ) - if unseen not in accepted_values: + if not isinstance(unseen, str) or unseen not in accepted_values: raise ValueError( f"Parameter `unseen` takes only values {', '.join(accepted_values)}." f" Got {unseen} instead." ) + + +def add_target_to_X(nw_X, y): + """Add y to X as the column TARGET_NAME, pairing rows by position. + + y can be a series, list or array. With pandas, the column takes the index of X. + """ + if nwd.is_into_series(y): + y_nw = nw.from_native(y, series_only=True) + else: + y_nw = nw.new_series(name=TARGET_NAME, values=y, backend=nw_X.implementation) + return nw_X.with_columns(y_nw.alias(TARGET_NAME)) diff --git a/feature_engine/encoding/base_encoder.py b/feature_engine/encoding/base_encoder.py index adf856dbd..77ebbb5d5 100644 --- a/feature_engine/encoding/base_encoder.py +++ b/feature_engine/encoding/base_encoder.py @@ -96,7 +96,10 @@ def __init__( ignore_format: bool = False, ) -> None: - if missing_values not in ["raise", "ignore"]: + if not isinstance(missing_values, str) or missing_values not in [ + "raise", + "ignore", + ]: raise ValueError( "missing_values takes only values 'raise' or 'ignore'. " f"Got {missing_values} instead." diff --git a/feature_engine/encoding/count_frequency.py b/feature_engine/encoding/count_frequency.py index 3d063c7dc..317b1ca1b 100644 --- a/feature_engine/encoding/count_frequency.py +++ b/feature_engine/encoding/count_frequency.py @@ -189,7 +189,10 @@ def __init__( unseen: str = "ignore", ) -> None: - if encoding_method not in ["count", "frequency"]: + if not isinstance(encoding_method, str) or encoding_method not in [ + "count", + "frequency", + ]: raise ValueError( "encoding_method takes only values 'count' and 'frequency'. " f"Got {encoding_method} instead." diff --git a/feature_engine/encoding/mean_encoding.py b/feature_engine/encoding/mean_encoding.py index 71aead003..10b223e5f 100644 --- a/feature_engine/encoding/mean_encoding.py +++ b/feature_engine/encoding/mean_encoding.py @@ -30,7 +30,11 @@ ) from feature_engine._docstrings.substitute import Substitution from feature_engine.dataframe_checks import check_X_y -from feature_engine.encoding._helper_functions import check_parameter_unseen +from feature_engine.encoding._helper_functions import ( + TARGET_NAME, + add_target_to_X, + check_parameter_unseen, +) from feature_engine.encoding.base_encoder import ( CategoricalInitMixinNA, CategoricalMethodsMixin, @@ -225,16 +229,8 @@ def fit(self, X: IntoDataFrame, y: IntoSeries): self.encoder_dict_ = {} - # pair y with X by position, so list, array and series targets all work - target_name = "__feature_engine_mean_target__" - if nwd.is_into_series(y): - y_nw = nw.from_native(y, series_only=True).alias(target_name) - else: - y_nw = nw.new_series( - name=target_name, values=y, backend=nw_X.implementation - ) - nw_Xy = nw_X.with_columns(y_nw) - + nw_Xy = add_target_to_X(nw_X, y) + y_nw = nw_Xy[TARGET_NAME] y_prior = y_nw.mean() if self.unseen == "encode": @@ -246,7 +242,7 @@ def fit(self, X: IntoDataFrame, y: IntoSeries): # pandas is faster than narwhals. if nwd.is_pandas_dataframe(X): # pandas series with the index of X - y = nw_Xy[target_name].to_native() + y = y_nw.to_native() for var in variables_: if self.smoothing == "auto": damping = y.groupby(X[var]).var(ddof=0) / y_var @@ -262,9 +258,9 @@ def fit(self, X: IntoDataFrame, y: IntoSeries): else: for var in variables_: stats = nw_Xy.group_by(var, drop_null_keys=True).agg( - nw.col(target_name).mean().alias("__mean__"), - nw.col(target_name).len().alias("__count__"), - nw.col(target_name).var(ddof=0).alias("__var__"), + nw.col(TARGET_NAME).mean().alias("__mean__"), + nw.col(TARGET_NAME).len().alias("__count__"), + nw.col(TARGET_NAME).var(ddof=0).alias("__var__"), ) if self.smoothing == "auto": damping = nw.col("__var__") / y_var diff --git a/tests/test_encoding/test_base_encoders/test_categorical_init_mixin.py b/tests/test_encoding/test_base_encoders/test_categorical_init_mixin.py index 2c6e65fb4..daada0742 100644 --- a/tests/test_encoding/test_base_encoders/test_categorical_init_mixin.py +++ b/tests/test_encoding/test_base_encoders/test_categorical_init_mixin.py @@ -1,3 +1,5 @@ +import re + import pytest from feature_engine.encoding.base_encoder import CategoricalInitMixin @@ -5,10 +7,9 @@ @pytest.mark.parametrize("param", [1, "hola", [1, 2, 0], (True, False)]) def test_raises_error_when_ignore_format_not_permitted(param): - with pytest.raises(ValueError) as record: - CategoricalInitMixin(ignore_format=param) msg = f"ignore_format takes only booleans True and False. Got {param} instead." - assert str(record.value) == msg + with pytest.raises(ValueError, match=re.escape(msg)): + CategoricalInitMixin(ignore_format=param) @pytest.mark.parametrize("param", [True, False]) diff --git a/tests/test_encoding/test_base_encoders/test_categorical_init_mixin_na.py b/tests/test_encoding/test_base_encoders/test_categorical_init_mixin_na.py index a7c112816..4eaebb376 100644 --- a/tests/test_encoding/test_base_encoders/test_categorical_init_mixin_na.py +++ b/tests/test_encoding/test_base_encoders/test_categorical_init_mixin_na.py @@ -1,3 +1,5 @@ +import re + import pytest from feature_engine.encoding.base_encoder import CategoricalInitMixinNA @@ -5,18 +7,18 @@ @pytest.mark.parametrize("param", [1, "hola", [1, 2, 0], (True, False)]) def test_raises_error_when_ignore_format_not_permitted(param): - with pytest.raises(ValueError) as record: - CategoricalInitMixinNA(ignore_format=param) msg = f"ignore_format takes only booleans True and False. Got {param} instead." - assert str(record.value) == msg + with pytest.raises(ValueError, match=re.escape(msg)): + CategoricalInitMixinNA(ignore_format=param) -@pytest.mark.parametrize("param", [1, "hola", [1, 2, 0], (True, False)]) +@pytest.mark.parametrize( + "param", [1, "hola", "Raise", None, [1, 2, 0], ["raise"], (True, False)] +) def test_raises_error_when_missing_values_not_permitted(param): - with pytest.raises(ValueError) as record: - CategoricalInitMixinNA(missing_values=param) msg = f"missing_values takes only values 'raise' or 'ignore'. Got {param} instead." - assert str(record.value) == msg + with pytest.raises(ValueError, match=re.escape(msg)): + CategoricalInitMixinNA(missing_values=param) @pytest.mark.parametrize("param", [(True, "ignore"), (False, "raise")]) diff --git a/tests/test_encoding/test_count_frequency_encoder.py b/tests/test_encoding/test_count_frequency_encoder.py index b7445f6c7..0b166f5bd 100644 --- a/tests/test_encoding/test_count_frequency_encoder.py +++ b/tests/test_encoding/test_count_frequency_encoder.py @@ -18,13 +18,16 @@ # init parameters -@pytest.mark.parametrize("enc_method", ["arbitrary", False, 1]) +@pytest.mark.parametrize( + "enc_method", + ["arbitrary", "Count", "", False, 1, None, ["count"], ("frequency",)], +) def test_error_if_encoding_method_not_permitted_value(enc_method): msg = ( "encoding_method takes only values 'count' and 'frequency'. " f"Got {enc_method} instead." ) - with pytest.raises(ValueError, match=msg): + with pytest.raises(ValueError, match=re.escape(msg)): CountEncoder(encoding_method=enc_method) @@ -337,18 +340,27 @@ def test_inverse_transform_when_encode_unseen(make_df): def test_inverse_transform_raises_non_fitted_error(make_df): df1 = make_df({"words": ["dog", "dog", "cat", "cat", "cat", "bird"]}) enc = CountEncoder() + msg = ( + "This CountEncoder instance is not fitted yet. Call 'fit' with " + "appropriate arguments before using this estimator." + ) + msg_na = ( + "Some of the variables in the dataset contain NaN. Check and " + "remove those before using this transformer or set the parameter " + "`missing_values='ignore'` when initialising this transformer." + ) # Test when fit is not called prior to transform. - with pytest.raises(NotFittedError): + with pytest.raises(NotFittedError, match=re.escape(msg)): enc.inverse_transform(df1) df1_na = make_df({"words": ["dog", "dog", "cat", "cat", "cat", None]}) - with pytest.raises(ValueError): + with pytest.raises(ValueError, match=re.escape(msg_na)): enc.fit(df1_na) # Test when fit is not called prior to transform. - with pytest.raises(NotFittedError): + with pytest.raises(NotFittedError, match=re.escape(msg)): enc.inverse_transform(df1_na) diff --git a/tests/test_encoding/test_helper_functions.py b/tests/test_encoding/test_helper_functions.py index 022c051c3..6616cf2ef 100644 --- a/tests/test_encoding/test_helper_functions.py +++ b/tests/test_encoding/test_helper_functions.py @@ -1,22 +1,49 @@ +import re + +import narwhals as nw +import numpy as np +import pandas as pd import pytest -from feature_engine.encoding._helper_functions import check_parameter_unseen +from feature_engine.encoding._helper_functions import ( + TARGET_NAME, + add_target_to_X, + check_parameter_unseen, +) +from tests.backend_helpers import frame_to_dict, make_series @pytest.mark.parametrize("accepted", ["one", False, [1, 2], ("one", "two"), 1]) def test_raises_error_when_accepted_values_not_permitted(accepted): - with pytest.raises(ValueError) as record: - check_parameter_unseen("zero", accepted) msg = "accepted_values should be a list of strings. " f" Got {accepted} instead." - assert str(record.value) == msg + with pytest.raises(ValueError, match=re.escape(msg)): + check_parameter_unseen("zero", accepted) -@pytest.mark.parametrize("accepted", [["one", "two"], ["three", "four"]]) -def test_raises_error_when_error_not_in_accepted_values(accepted): - with pytest.raises(ValueError) as record: - check_parameter_unseen("zero", accepted) - msg = ( - f"Parameter `unseen` takes only values {', '.join(accepted)}." - " Got zero instead." - ) - assert str(record.value) == msg +@pytest.mark.parametrize("unseen", ["zero", "One", "", 1, None, ["one"], ("one",)]) +def test_raises_error_when_unseen_not_in_accepted_values(unseen): + msg = f"Parameter `unseen` takes only values one, two. Got {unseen} instead." + with pytest.raises(ValueError, match=re.escape(msg)): + check_parameter_unseen(unseen, ["one", "two"]) + + +@pytest.mark.parametrize("to_target", [list, np.array, "series"]) +def test_add_target_to_X_pairs_rows_by_position(make_df, to_target): + X = make_df({"var_A": ["a", "b", "c"]}) + values = [1, 0, 1] + if to_target == "series": + y = make_series(make_df, values) + else: + y = to_target(values) + + Xy = add_target_to_X(nw.from_native(X), y).to_native() + + assert isinstance(Xy, make_df) + assert frame_to_dict(Xy) == {"var_A": ["a", "b", "c"], TARGET_NAME: values} + + +def test_add_target_to_X_keeps_the_pandas_index(): + X = pd.DataFrame({"var_A": ["a", "b", "c"]}, index=[12, 10, 11]) + Xy = add_target_to_X(nw.from_native(X), np.array([1, 0, 1])).to_native() + assert Xy.index.tolist() == [12, 10, 11] + assert Xy[TARGET_NAME].tolist() == [1, 0, 1] diff --git a/tests/test_encoding/test_mean_encoder.py b/tests/test_encoding/test_mean_encoder.py index 43237d4bb..164e27e23 100644 --- a/tests/test_encoding/test_mean_encoder.py +++ b/tests/test_encoding/test_mean_encoder.py @@ -12,31 +12,47 @@ ENC_DICT_VAR_B = {"A": 0.2, "B": 0.3333333333333333, "C": 0.5} -# test init params -@pytest.mark.parametrize("params", [("raise", True, "auto"), ("ignore", False, 1)]) -def test_init_param_assignment(params): - MeanEncoder( - missing_values=params[0], - ignore_format=params[1], - unseen=params[0], - smoothing=params[2], - ) - - +# init parameters @pytest.mark.parametrize( - "errors", ["empanada", False, 1, ("raise", "ignore"), ["ignore"]] + "unseen", ["empanada", False, 1, None, ("raise", "ignore"), ["ignore"]] ) -def test_error_if_unseen_gets_not_permitted_value(errors): - with pytest.raises(ValueError): - MeanEncoder(unseen=errors) +def test_error_if_unseen_gets_not_permitted_value(unseen): + msg = ( + "Parameter `unseen` takes only values ignore, raise, encode. " + f"Got {unseen} instead." + ) + with pytest.raises(ValueError, match=re.escape(msg)): + MeanEncoder(unseen=unseen) @pytest.mark.parametrize("smoothing", ["hello", ["auto"], -1]) def test_raises_error_when_not_allowed_smoothing_param_in_init(smoothing): - with pytest.raises(ValueError): + msg = f"smoothing must be greater than 0 or 'auto'. Got {smoothing} instead." + with pytest.raises(ValueError, match=re.escape(msg)): MeanEncoder(smoothing=smoothing) +@pytest.mark.parametrize( + "missing_values, ignore_format, unseen, smoothing", + [ + ("raise", True, "ignore", "auto"), + ("ignore", False, "encode", 1), + ("raise", False, "raise", 0.5), + ], +) +def test_init_param_assignment(missing_values, ignore_format, unseen, smoothing): + encoder = MeanEncoder( + missing_values=missing_values, + ignore_format=ignore_format, + unseen=unseen, + smoothing=smoothing, + ) + assert encoder.missing_values == missing_values + assert encoder.ignore_format is ignore_format + assert encoder.unseen == unseen + assert encoder.smoothing == smoothing + + # fit and transform def test_user_enters_1_variable(make_df, data_enc): # test case 1: 1 variable @@ -358,16 +374,25 @@ def test_inverse_transform_raises_non_fitted_error(make_df): df1 = make_df({"words": ["dog", "dog", "cat", "cat", "cat", "bird"]}) y = make_series(make_df, [1, 0, 1, 0, 1, 0]) enc = MeanEncoder() + msg = ( + "This MeanEncoder instance is not fitted yet. Call 'fit' with " + "appropriate arguments before using this estimator." + ) + msg_na = ( + "Some of the variables in the dataset contain NaN. Check and " + "remove those before using this transformer or set the parameter " + "`missing_values='ignore'` when initialising this transformer." + ) # Test when fit is not called prior to transform. - with pytest.raises(NotFittedError): + with pytest.raises(NotFittedError, match=re.escape(msg)): enc.inverse_transform(df1) df1_na = make_df({"words": ["dog", "dog", "cat", "cat", "cat", None]}) - with pytest.raises(ValueError): + with pytest.raises(ValueError, match=re.escape(msg_na)): enc.fit(df1_na, y) # Test when fit is not called prior to transform. - with pytest.raises(NotFittedError): + with pytest.raises(NotFittedError, match=re.escape(msg)): enc.inverse_transform(df1_na) From 2f41406bf9132545559ec2cdc61d872af5ea5587 Mon Sep 17 00:00:00 2001 From: Soledad Galli Date: Tue, 15 Sep 2026 13:46:04 +0200 Subject: [PATCH 2/2] Fix get_feature_names_out error message, check missing_values type Co-Authored-By: Claude Opus 5 --- feature_engine/_base_transformers/mixins.py | 2 +- .../check_init_input_params.py | 5 ++++- .../test_get_feature_names_out_mixin.py | 8 ++++++-- .../test_check_init_input_params.py | 18 +++++++++++++++--- 4 files changed, 26 insertions(+), 7 deletions(-) diff --git a/feature_engine/_base_transformers/mixins.py b/feature_engine/_base_transformers/mixins.py index 8c6a0a66c..b76481e78 100644 --- a/feature_engine/_base_transformers/mixins.py +++ b/feature_engine/_base_transformers/mixins.py @@ -164,7 +164,7 @@ def get_feature_names_out( else: raise ValueError( "input_features must be a list or an array. " - "Got {input_features} instead." + f"Got {input_features} instead." ) feature_names = self.feature_names_in_ diff --git a/feature_engine/_check_init_parameters/check_init_input_params.py b/feature_engine/_check_init_parameters/check_init_input_params.py index e1000accc..a89b3b500 100644 --- a/feature_engine/_check_init_parameters/check_init_input_params.py +++ b/feature_engine/_check_init_parameters/check_init_input_params.py @@ -1,5 +1,8 @@ def _check_param_missing_values(missing_values): - if missing_values not in ["raise", "ignore"]: + if not isinstance(missing_values, str) or missing_values not in [ + "raise", + "ignore", + ]: raise ValueError( "missing_values takes only values 'raise' or 'ignore'. " f"Got {missing_values} instead." diff --git a/tests/test_base_transformers/test_get_feature_names_out_mixin.py b/tests/test_base_transformers/test_get_feature_names_out_mixin.py index 7315b694b..2cfb83c91 100644 --- a/tests/test_base_transformers/test_get_feature_names_out_mixin.py +++ b/tests/test_base_transformers/test_get_feature_names_out_mixin.py @@ -1,3 +1,5 @@ +import re + import numpy as np import pandas as pd import polars as pl @@ -155,10 +157,12 @@ def test_raise_error_when_input_feature_non_permitted(): with pytest.raises(ValueError, match="feature_names_in_"): transformer.get_feature_names_out(input_features=np.array(["Name", "Age"])) - with pytest.raises(ValueError, match="list or an array"): + msg = "input_features must be a list or an array. Got var1 instead." + with pytest.raises(ValueError, match=re.escape(msg)): transformer.get_feature_names_out(input_features="var1") - with pytest.raises(ValueError, match="list or an array"): + msg = "input_features must be a list or an array. Got True instead." + with pytest.raises(ValueError, match=re.escape(msg)): transformer.get_feature_names_out(input_features=True) diff --git a/tests/test_check_init_parameters/test_check_init_input_params.py b/tests/test_check_init_parameters/test_check_init_input_params.py index 4f4b7f631..d2c08abff 100644 --- a/tests/test_check_init_parameters/test_check_init_input_params.py +++ b/tests/test_check_init_parameters/test_check_init_input_params.py @@ -1,3 +1,5 @@ +import re + import pytest from feature_engine._check_init_parameters.check_init_input_params import ( @@ -6,13 +8,23 @@ ) -@pytest.mark.parametrize("missing_vals", [None, ["Hola"], True, "Hola"]) +@pytest.mark.parametrize( + "missing_vals", [None, ["Hola"], ["raise"], ("ignore",), True, 1, "Hola", "Raise"] +) def test_check_param_missing_values(missing_vals): - with pytest.raises(ValueError): + msg = ( + "missing_values takes only values 'raise' or 'ignore'. " + f"Got {missing_vals} instead." + ) + with pytest.raises(ValueError, match=re.escape(msg)): _check_param_missing_values(missing_vals) @pytest.mark.parametrize("drop_orig", [None, ["Hola"], 10, "Hola"]) def test_check_param_drop_original(drop_orig): - with pytest.raises(ValueError): + msg = ( + "drop_original takes only boolean values True and False. " + f"Got {drop_orig} instead." + ) + with pytest.raises(ValueError, match=re.escape(msg)): _check_param_drop_original(drop_orig)