Repository: scikit-learn/scikit-learn
Pull request: [MRG]Fix simple imputer for string input
#### Reference Issues/PRs
Fixes #17525

#### What does this implement/fix?
* Fix to accept list of strings in ```SimpleImputer``` with ```strategy```="most_frequent" or "constant"
* Add unittest

#### Any other comments?
* If this PR is not proper, please tell me. I will close:)

Problem statement:
ValueError in SimpleImputer for string input with strategy='most_frequent'/'constant'
#### Describe the bug
* List of strings raise ValueError in SimpleImputer with strategy='most_frequent' (also 'constant')

#### Steps/Code to Reproduce

```python
import numpy as np
from sklearn.impute import SimpleImputer


X = [['a', 'b', 'c'], ['d', 'e', np.nan]]

imp_mf = SimpleImputer(missing_values=np.nan, strategy='most_frequent')
transformed_mf = imp_mf.fit_transform(X)
print(transformed_mf)
```

#### Expected Results
```
[['a' 'b' 'c']
 ['d' 'e' 'c']]
```
#### Actual Results
```
--------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-10-6cee172813bf> in <module>()
      6 
      7 imp_mf = SimpleImputer(missing_values=np.nan, strategy='most_frequent')
----> 8 transformed_mf = imp_mf.fit_transform(X)
      9 print(transformed_mf)

2 frames
/content/scikit-learn/sklearn/impute/_base.py in _validate_input(self, X, in_fit)
    258                              "categorical data represented either as an array "
    259                              "with integer dtype or an array of string values "
--> 260                              "with an object dtype.".format(X.dtype))
    261 
    262         return X

ValueError: SimpleImputer does not support data with dtype <U3. Please provide either a numeric array (with a floating point or integer dtype) or categorical data represented either as an array with integer dtype or an array of string values with an object dtype..
```

#### Versions
```
System:
    python: 3.6.9 (default, Apr 18 2020, 01:56:04)  [GCC 8.4.0]
executable: /usr/bin/python3
   machine: Linux-4.19.104+-x86_64-with-Ubuntu-18.04-bionic

Python dependencies:
          pip: 19.3.1
   setuptools: 47.1.1
      sklearn: 0.23.1
        numpy: 1.18.4
        scipy: 1.4.1
       Cython: 0.29.19
       pandas: 1.0.4
   matplotlib: 3.2.1
       joblib: 0.15.1
threadpoolctl: 2.1.0

Built with OpenMP: True
```

Hints from issue discussion:
I can reproduce the error. Since the documentation says that `fit` and `transform` methods allow `array-like` (`list` or `ndarray`) as input, I would assume that this is a bug.
@alfaro96 Thank you for your comment!

I tried to fix the issue at PR #17526. 
I would be grateful if you could check the PR when you have time. m(_ _)m

Any suggestion is welcomed:)

Commit message: Fix highlight in change log

Patch to review:
diff --git a/doc/whats_new/v0.24.rst b/doc/whats_new/v0.24.rst
index 848b415890452..9cd8902f0643f 100644
--- a/doc/whats_new/v0.24.rst
+++ b/doc/whats_new/v0.24.rst
@@ -61,6 +61,10 @@ Changelog
   change since `None` was defaulting to these values already.
   :pr:`16493` by :user:`Darshan N <DarshanGowda0>`.
 
+- |Feature| :class:`impute.SimpleImputer` now supports a list of strings
+  when ``strategy='most_frequent'`` or ``strategy='constant'``.
+  :pr:`17526` by :user:`Ayako YAGI <yagi-3>` and :user:`Juan Carlos Alfaro Jiménez <alfaro96>`.
+
 :mod:`sklearn.metrics`
 ......................
 
diff --git a/sklearn/impute/_base.py b/sklearn/impute/_base.py
index 517de982d8478..cedc97872333c 100644
--- a/sklearn/impute/_base.py
+++ b/sklearn/impute/_base.py
@@ -228,7 +228,15 @@ def _validate_input(self, X, in_fit):
                                                         self.strategy))
 
         if self.strategy in ("most_frequent", "constant"):
-            dtype = None
+            # If input is a list of strings, dtype = object.
+            # Otherwise ValueError is raised in SimpleImputer
+            # with strategy='most_frequent' or 'constant'
+            # because the list is converted to Unicode numpy array
+            if isinstance(X, list) and \
+               any(isinstance(elem, str) for row in X for elem in row):
+                dtype = object
+            else:
+                dtype = None
         else:
             dtype = FLOAT_DTYPES
 
diff --git a/sklearn/impute/tests/test_impute.py b/sklearn/impute/tests/test_impute.py
index 960f671915e6a..2da6b2a977af1 100644
--- a/sklearn/impute/tests/test_impute.py
+++ b/sklearn/impute/tests/test_impute.py
@@ -351,6 +351,21 @@ def test_imputation_most_frequent_pandas(dtype):
     assert_array_equal(X_trans, X_true)
 
 
+def test_imputation_most_frequent_string_list():
+    X = [['a', 'b'],
+         ['c', np.nan]]
+
+    X_true = np.array([
+        ['a', 'b'],
+        ['c', 'b']
+    ], dtype=object)
+
+    imputer = SimpleImputer(strategy="most_frequent")
+    X_trans = imputer.fit_transform(X)
+
+    assert_array_equal(X_trans, X_true)
+
+
 @pytest.mark.parametrize("X_data, missing_value", [(1, 0), (1., np.nan)])
 def test_imputation_constant_error_invalid_type(X_data, missing_value):
     # Verify that exceptions are raised on invalid fill_value type
@@ -464,6 +479,21 @@ def test_imputation_constant_pandas(dtype):
     assert_array_equal(X_trans, X_true)
 
 
+def test_imputation_constant_string_list():
+    X = [['a', 'b'],
+         ['c', np.nan]]
+
+    X_true = np.array([
+        ['a', 'b'],
+        ['c', 'missing_value']
+    ], dtype=object)
+
+    imputer = SimpleImputer(strategy="constant")
+    X_trans = imputer.fit_transform(X)
+
+    assert_array_equal(X_trans, X_true)
+
+
 @pytest.mark.parametrize("X", [[[1], [2]], [[1], [np.nan]]])
 def test_iterative_imputer_one_feature(X):
     # check we exit early when there is a single feature


Code review comments:
diff_hunk: @@ -351,6 +351,21 @@ def test_imputation_most_frequent_pandas(dtype):
     assert_array_equal(X_trans, X_true)
 
 
+def test_imputation_most_frequent_string_list():
+    X = [['a', 'b'],
+         ['c', np.nan]]
+
+    X_true = np.array([
+        ['a', 'b'],
+        ['c', 'b']
+    ], dtype=object)
+
+    imputer = SimpleImputer(strategy="most_frequent")
path: sklearn/impute/tests/test_impute.py
text: @user2:
```suggestion
@user1.mark.parametrize(
    'strategy, expected',
    [('most_frequent', 'b'), ('constant', 'missing_value')]
)
def test_imputation_most_frequent_string_list(strategy, expected):
    X = [['a', 'b'],
         ['c', np.nan]]

    X_true = np.array([
        ['a', 'b'],
        ['c', expected]
    ], dtype=object)

    imputer = SimpleImputer(strategy=strategy)
```

and remove the other test.

@author:
@user2 Thank you! I followed your suggestion:)

In addition, following little modifications are done.
* `test_imputation_constant_string_list` is removed (because it is done by parameterized new unittest)
* The unittest name is changed to `test_simple_imputation_string_list ` (which is combined  `test_imputation_most_frequent_string_list` and `test_imputation_constant_string_list `)