Repository: scikit-learn/scikit-learn
Pull request: ENH Adds feature_name_combiner to OneHotEncoder
This change makes sure that `OneHotEncoder.get_feature_names_out` returns unique feature names, as every transformer should do. The approach is different than discussed in the issue, erring on the side of working by default, rather than forcing the user to provide a custom function that transforms (feature_name, value) into the feature name of the encoded column. It's less flexible, but works without user's intervention.

Fixes https://github.com/scikit-learn/scikit-learn/issues/22488.

I'm happy to chance the approach here, but I'd expect this being good enough for majority of use cases.

---

This also changes the behaviour of the case explained in https://github.com/scikit-learn/scikit-learn/issues/16593. Instead of raising by accident on impossible `int + str`, we cast the feature name to `str` - we're producing string feature names after all. The exception was accidental.

I'm not sure whether we should close that other issue because there's a discussion on whether we should raise if column names are integers.

One could argue that we should remove the str(feature_name) piece from this PR.

I think we shouldn't because this change enables them to use OneHotEncoder without renaming fields. I suspect they have integer feature names because they don't care about interpreting them after the pipeline, at least until the expectation of feature names is clearly set.

<!--
Thanks for contributing a pull request! Please ensure you have taken a look at
the contribution guidelines: https://github.com/scikit-learn/scikit-learn/blob/main/CONTRIBUTING.md
-->

#### Reference Issues/PRs
Fixes https://github.com/scikit-learn/scikit-learn/issues/22488
See also https://github.com/scikit-learn/scikit-learn/issues/16593
Forces a rebase for https://github.com/scikit-learn/scikit-learn/pull/21754
<!--
Example: Fixes #1234. See also #3456.
Please use keywords (e.g., Fixes) to create link to the issues or pull requests
you resolved, so that they will automatically be closed when your pull request
is merged. See https://github.com/blog/1506-closing-issues-via-pull-requests
-->


Problem statement:
OneHotEncoder creates non unique column names, when values None (NoneType) and "None" (str) are both present
### Describe the bug

OneHotEncoder creates non unique column names when values None (NoneType) and "None" (str) are both present in the same column.

Such policy creates downstream problems (e.g. ColumnTransformer raising because of non unique column names).

It's not 100% clear to me what's the expectation of a transformer.
Should every transformer emit unique names for features?
Why does the ColumnTransformer raise if there are non-unique names?

### Steps/Code to Reproduce

Just one hot encoder.
```py
from sklearn.preprocessing import OneHotEncoder

X = [['None'], [None]]
t = OneHotEncoder().fit(X)
feature_names = t.get_feature_names_out()
# ['x0_None' 'x0_None']

assert len(feature_names) == len(set(feature_names))
```


With ColumnTransformer
```py
import pandas as pd
from sklearn.compose import make_column_selector, make_column_transformer
from sklearn.preprocessing import OneHotEncoder

t = make_column_transformer(
    (OneHotEncoder(), make_column_selector("x0")),
    verbose_feature_names_out=False,
)
t.fit(pd.DataFrame({"x0": [None, "None"]}))
print(t.get_feature_names_out(["x0"]))
```

There's a subtle bug in the ColumnTransformer as well. If `verbose_feature_names_out` is True, it will not check names and will emit non-unique names without raising.

### Expected Results

I'd expect different feature names being generated.

### Actual Results

First code snippet
```py
['x0_None' 'x0_None']
```

Second code snippet
```py
Traceback (most recent call last):
  File "src/ohe_bug.py", line 10, in <module>
    print(t.get_feature_names_out(["x0"]))
  File "/home/pilotuser/.local/lib/python3.8/site-packages/sklearn/compose/_column_transformer.py", line 518, in get_feature_names_out
    raise ValueError(
ValueError: Output feature names: ['x0_None'] are not unique. Please set verbose_feature_names_out=True to add prefixes to feature names
```

### Versions

```shell
System:
    python: 3.8.12 (default, Sep 28 2021, 19:23:30)  [GCC 8.3.0]
executable: /usr/local/bin/python
   machine: Linux-4.14.238-182.422.amzn2.x86_64-x86_64-with-glibc2.2.5

Python dependencies:
          pip: 21.2.4
   setuptools: 46.0.0
      sklearn: 1.0.2
        numpy: 1.18.5
        scipy: 1.5.4
       Cython: None
       pandas: 1.1.1
   matplotlib: 3.2.2
       joblib: 1.0.1
threadpoolctl: 2.1.0

Built with OpenMP: True
```
```

Hints from issue discussion:
Indeed, the expected behaviour would be to get 2 different names there.
I could imagine that we could have the same bug with names reserved in Python (e.g. True vs "True").

It should be possible to detect the collision since the `repr` of `None` and `"None"` will differ.
However, I don't know if we should error or make some magic under the hood.
@glemaitre that's a good point on repr. 

I've gone around the issue (in private codebase) by prepending the `type(v).__name__`, but it's not great since it adds many characters, and often very little/no information. It's not bulletproof by any means (two types could have the same name).

Another way to go around it would be to allow passing a function to transform `(column, value)` tuple into feature name. The default implementation would be `feature_name + "_" + str(v)`, but the consumer could override it with something else (e.g. using repr, adding type, anything). That seems too generic and something that wouldn't be used very often.

How would a change from `str` to `repr` be treated? Would it be considered as a breaking change?

> However, I don't know if we should error or make some magic under the hood.

Is there a clear expectation from the transformer, regarding feature names? Should they be unique? 

> It should be possible to detect the collision since the repr of None and "None" will differ.

That's true. Given that OneHotEncoder knows all the values after `fit` finishes, it could apply different logic if there are some collisions. It does seem a bit hacky, though - it'd encode some values differently based on whether there are collisions or not.

Btw I could do the fix once we agree on the direction.
I think the suggested solution for a callable in `ColumnTransformer` would be the most flexible. It would also help resolve other issues such as https://github.com/scikit-learn/scikit-learn/issues/16593
@thomasjpfan I'll prepare the PR today or tomorrow so we have a starting implementation to discuss.
I've created a PR linked above (https://github.com/scikit-learn/scikit-learn/pull/22506) that provides one way to fix it.

Commit message: Update sklearn/preprocessing/_encoders.py

Co-authored-by: Christian Lorentzen <lorentzen.ch@gmail.com>

Patch to review:
diff --git a/doc/whats_new/v1.1.rst b/doc/whats_new/v1.1.rst
index f6dc1f311ad3c..97238b6211ea6 100644
--- a/doc/whats_new/v1.1.rst
+++ b/doc/whats_new/v1.1.rst
@@ -26,7 +26,7 @@ Changelog
   classifier that always predicts the positive class: recall=100% and
   precision=class balance.
   :pr:`23214` by :user:`Stéphane Collot <stephanecollot>` and :user:`Max Baak <mbaak>`.
-  
+
 :mod:`sklearn.utils`
 ....................
 
@@ -208,7 +208,7 @@ Changelog
   :pr:`23194` by `Thomas Fan`_.
 
 - |Enhancement| Added an extension in doc/conf.py to automatically generate
-  the list of estimators that handle NaN values. 
+  the list of estimators that handle NaN values.
   :pr:`23198` by `Lise Kleiber <lisekleiber>`_, :user:`Zhehao Liu <MaxwellLZH>`
   and :user:`Chiara Marmo <cmarmo>`.
 
diff --git a/doc/whats_new/v1.2.rst b/doc/whats_new/v1.2.rst
index e409461bccc77..b2df3e0535347 100644
--- a/doc/whats_new/v1.2.rst
+++ b/doc/whats_new/v1.2.rst
@@ -50,6 +50,15 @@ Changelog
 - |Efficiency| Improve runtime performance of :class:`ensemble.IsolationForest`
   by avoiding data copies. :pr:`23252` by :user:`Zhehao Liu <MaxwellLZH>`.
 
+:mod:`sklearn.preprocessing`
+...........................
+
+- |Enhancement| Adds a `feature_name_combiner` parameter to
+  :class:`preprocessing.OneHotEncoder`. This specifies a custom callable to create
+  feature names to be returned by :meth:`get_feature_names_out`.
+  The callable combines input arguments `(input_feature, category)` to a string.
+  :pr:`22506` by :user:`Mario Kostelac <mariokostelac>`.
+
 Code and Documentation Contributors
 -----------------------------------
 
diff --git a/sklearn/preprocessing/_encoders.py b/sklearn/preprocessing/_encoders.py
index d4cc642a18562..016ac349abb49 100644
--- a/sklearn/preprocessing/_encoders.py
+++ b/sklearn/preprocessing/_encoders.py
@@ -319,6 +319,13 @@ class OneHotEncoder(_BaseEncoder):
         .. versionadded:: 1.1
             Read more in the :ref:`User Guide <one_hot_encoder_infrequent_categories>`.
 
+    feature_name_combiner : {'concat_string'} or Callable, default='concat_string'
+        Callable with signature `def callable(input_feature, category)` that returns a string.
+        This is used to create feature names to be returned by :meth:`get_feature_names_out`.
+
+        'concat_string' concatenates encoded feature name and category separated by
+        '_'.  E.g. feature X with values 1, 6, 7 create feature names X_1, X_6, X_7.
+
     Attributes
     ----------
     categories_ : list of arrays
@@ -418,6 +425,14 @@ class OneHotEncoder(_BaseEncoder):
     array([[0., 1., 0., 0.],
            [1., 0., 1., 0.]])
 
+    One can change the way feature names are created.
+
+    >>> def custom_combiner(feature, category):
+    ...     return str(feature) + "_" + type(category).__name__ + "_" + str(category)
+    >>> custom_fnames_enc = OneHotEncoder(feature_name_combiner=custom_combiner).fit(X)
+    >>> custom_fnames_enc.get_feature_names_out()
+    array(['x0_str_Female', 'x0_str_Male', 'x1_int_1', 'x1_int_2', 'x1_int_3'],
+          dtype=object)
     Infrequent categories are enabled by setting `max_categories` or `min_frequency`.
 
     >>> import numpy as np
@@ -438,6 +453,7 @@ def __init__(
         sparse=True,
         dtype=np.float64,
         handle_unknown="error",
+        feature_name_combiner="concat_string",
         min_frequency=None,
         max_categories=None,
     ):
@@ -446,6 +462,7 @@ def __init__(
         self.dtype = dtype
         self.handle_unknown = handle_unknown
         self.drop = drop
+        self.feature_name_combiner = feature_name_combiner
         self.min_frequency = min_frequency
         self.max_categories = max_categories
 
@@ -467,6 +484,7 @@ def _validate_keywords(self):
                 f"'infrequent_if_exist' got {self.handle_unknown}."
             )
             raise ValueError(msg)
+        self._get_feature_name_combiner()  # call to force the validation
 
         if self.max_categories is not None and self.max_categories < 1:
             raise ValueError("max_categories must be greater than 1")
@@ -1072,12 +1090,12 @@ def get_feature_names(self, input_features=None):
                 "features ({}), got {}".format(len(cats), len(input_features))
             )
 
+        name_combiner = self._get_feature_name_combiner()
         feature_names = []
         for i in range(len(cats)):
-            names = [input_features[i] + "_" + str(t) for t in cats[i]]
+            names = [name_combiner(input_features[i], t) for t in cats[i]]
             feature_names.extend(names)
-
-        return np.array(feature_names, dtype=object)
+        return np.asarray(feature_names, dtype=object)
 
     def get_feature_names_out(self, input_features=None):
         """Get output feature names for transformation.
@@ -1106,13 +1124,25 @@ def get_feature_names_out(self, input_features=None):
             for i, _ in enumerate(self.categories_)
         ]
 
+        name_combiner = self._get_feature_name_combiner()
         feature_names = []
         for i in range(len(cats)):
-            names = [input_features[i] + "_" + str(t) for t in cats[i]]
+            names = [name_combiner(input_features[i], t) for t in cats[i]]
             feature_names.extend(names)
 
         return np.array(feature_names, dtype=object)
 
+    def _get_feature_name_combiner(self):
+        if self.feature_name_combiner == "concat_string":
+            return lambda feature, category: feature + "_" + str(category)
+        elif callable(self.feature_name_combiner):
+            return self.feature_name_combiner
+        else:
+            raise ValueError(
+                "feature_name_combiner has to be either 'concat_string' or callable,"
+                f" got {self.feature_name_combiner}"
+            )
+
 
 class OrdinalEncoder(_OneToOneFeatureMixin, _BaseEncoder):
     """
diff --git a/sklearn/preprocessing/tests/test_encoders.py b/sklearn/preprocessing/tests/test_encoders.py
index ea32de22cd2f0..f4677a5911561 100644
--- a/sklearn/preprocessing/tests/test_encoders.py
+++ b/sklearn/preprocessing/tests/test_encoders.py
@@ -208,6 +208,33 @@ def test_one_hot_encoder_feature_names_unicode(get_names):
     assert_array_equal(["n👍me_c❤t1", "n👍me_dat2"], feature_names)
 
 
+# TODO: Remove in 1.2 when get_feature_names is removed.
+@pytest.mark.filterwarnings("ignore::FutureWarning:sklearn")
+@pytest.mark.parametrize("get_names", ["get_feature_names", "get_feature_names_out"])
+def test_one_hot_encoder_custom_feature_name_combiner(get_names):
+    name_combiner = lambda feature, category: feature + "_" + repr(category)
+    enc = OneHotEncoder(feature_name_combiner=name_combiner)
+    X = np.array([["None", None]], dtype=object).T
+    enc.fit(X)
+    feature_names = getattr(enc, get_names)()
+    assert_array_equal(["x0_'None'", "x0_None"], feature_names)
+    feature_names = getattr(enc, get_names)(input_features=["a"])
+    assert_array_equal(["a_'None'", "a_None"], feature_names)
+
+
+# TODO: Remove pytest.parametrize in 1.2 when get_feature_names is removed.
+@pytest.mark.filterwarnings("ignore::FutureWarning:sklearn")
+@pytest.mark.parametrize("get_names", ["get_feature_names", "get_feature_names_out"])
+def test_one_hot_encoder_invalid_feature_name_combiner(get_names):
+    enc = OneHotEncoder(feature_name_combiner="invalid_preset")
+    X = np.array([[1, 2]], dtype=object).T
+    with pytest.raises(
+        ValueError,
+        match="feature_name_combiner has to be either 'concat_string' or callable,*",
+    ):
+        enc.fit(X)
+
+
 def test_one_hot_encoder_set_params():
     X = np.array([[1, 2]]).T
     oh = OneHotEncoder()


Code review comments:
diff_hunk: @@ -319,6 +319,13 @@ class OneHotEncoder(_BaseEncoder):
         .. versionadded:: 1.1
             Read more in the :ref:`User Guide <one_hot_encoder_infrequent_categories>`.
 
+    feature_name_combiner : {'concat_string'} or Callable, default='concat_string'
+        Callable with signature `def callable(input_feature, category)` that returns a string.
+        This is used to create feature names to be returned by :meth:`get_feature_names_out`.
+
+        'concat_string' concatenates encoded feature name and category separated by
+        '_'.  E.g. feature X with values 1, 6, 7 create feature names X_1, X_6, X_7.
path: sklearn/preprocessing/_encoders.py
text: @user1:
```suggestion
        '_'.  E.g. feature X with values 1, 6, 7 create feature names `X_1, X_6, X_7`.
```