Repository: scikit-learn/scikit-learn
Pull request: [MRG] BUG: add support for non numeric values in MissingIndicator
#### Reference Issues/PRs

closes #13035 

#### What does this implement/fix? Explain your changes.

Add support to non-numeric values in MissingIndicator. I flag this PR as BUG since SimpleImputer is already supporting non-numeric data and that we cannot use the MissingIndicator with the SimpleImputer in 0.20. I think that it should go in 0.20.3

I added 3 tests:

* check that we are raising an error when the numpy string type is raising an error;
* check that we can use string using object dtype;
* check that the missing indicator is working in conjunction with the SimpleImputer.


Problem statement:
MissingIndicator failed with non-numeric inputs

#### Description

```sklearn.Imputer.MissingIndicator``` fails with string and object type numpy arrays

#### String Types

##### Steps/Code to Reproduce 
```python
import numpy as np
from sklearn.impute import MissingIndicator

a = np.array([[c] for c in 'abcdea'], dtype=str)

MissingIndicator().fit_transform(a)
MissingIndicator(missing_values='a').fit_transform(a)
```

##### Expected Results

```
[[False]
 [False]
 [False]
 [False]
 [False]
 [False]]
[[False]
 [False]
 [True]
 [False]
 [False]
 [False]]
```

##### Actual Results

```
C:\Users\snowt\Python\scikit-learn\env\Scripts\python.exe C:/Users/snowt/Python/scikit-learn/test.py
[[False]
 [False]
 [False]
 [False]
 [False]
 [False]]
C:\Users\snowt\Python\scikit-learn\sklearn\utils\validation.py:558: FutureWarning: Beginning in version 0.22, arrays of bytes/strings will be converted to decimal numbers if dtype='numeric'. It is recommended that you convert the array to a float dtype before using it in scikit-learn, for example by using your_array = your_array.astype(np.float64).
  FutureWarning)
C:\Users\snowt\Python\scikit-learn\sklearn\utils\validation.py:558: FutureWarning: Beginning in version 0.22, arrays of bytes/strings will be converted to decimal numbers if dtype='numeric'. It is recommended that you convert the array to a float dtype before using it in scikit-learn, for example by using your_array = your_array.astype(np.float64).
  FutureWarning)
C:\Users\snowt\Python\scikit-learn\sklearn\utils\validation.py:558: FutureWarning: Beginning in version 0.22, arrays of bytes/strings will be converted to decimal numbers if dtype='numeric'. It is recommended that you convert the array to a float dtype before using it in scikit-learn, for example by using your_array = your_array.astype(np.float64).
  FutureWarning)
Traceback (most recent call last):
  File "C:/Users/snowt/Python/scikit-learn/test.py", line 7, in <module>
    print(MissingIndicator(missing_values='a').fit_transform(a))
  File "C:\Users\snowt\Python\scikit-learn\sklearn\impute.py", line 634, in fit_transform
    return self.fit(X, y).transform(X)
  File "C:\Users\snowt\Python\scikit-learn\sklearn\impute.py", line 570, in fit
    if self.features == 'missing-only'
  File "C:\Users\snowt\Python\scikit-learn\sklearn\impute.py", line 528, in _get_missing_features_info
    imputer_mask = _get_mask(X, self.missing_values)
  File "C:\Users\snowt\Python\scikit-learn\sklearn\impute.py", line 52, in _get_mask
    return np.equal(X, value_to_mask)
TypeError: ufunc 'equal' did not contain a loop with signature matching types dtype('<U1') dtype('<U1') dtype('bool')

Process finished with exit code 1
```

#### Object Types

##### Steps/Code to Reproduce 
```python
import numpy as np
from sklearn.impute import MissingIndicator

a = np.array([[c] for c in 'abcdea'], dtype=object)

MissingIndicator().fit_transform(a)
MissingIndicator(missing_values='a').fit_transform(a)
```

##### Expected Results

```
[[False]
 [False]
 [False]
 [False]
 [False]
 [False]]
[[False]
 [False]
 [True]
 [False]
 [False]
 [False]]
```

##### Actual Results

```
C:\Users\snowt\Python\scikit-learn\env\Scripts\python.exe C:/Users/snowt/Python/scikit-learn/test.py
Traceback (most recent call last):
  File "C:/Users/snowt/Python/scikit-learn/test.py", line 6, in <module>
    print(MissingIndicator().fit_transform(a))
  File "C:\Users\snowt\Python\scikit-learn\sklearn\impute.py", line 634, in fit_transform
    return self.fit(X, y).transform(X)
  File "C:\Users\snowt\Python\scikit-learn\sklearn\impute.py", line 555, in fit
    force_all_finite=force_all_finite)
  File "C:\Users\snowt\Python\scikit-learn\sklearn\utils\validation.py", line 522, in check_array
    array = np.asarray(array, dtype=dtype, order=order)
  File "C:\Users\snowt\Python\scikit-learn\env\lib\site-packages\numpy\core\numeric.py", line 538, in asarray
    return array(a, dtype, copy=False, order=order)
ValueError: could not convert string to float: 'a'

Process finished with exit code 1
```

#### Versions

```
System:
    python: 3.6.8 (tags/v3.6.8:3c6b436a57, Dec 24 2018, 00:16:47) [MSC v.1916 64
 bit (AMD64)]
executable: C:\Users\snowt\Python\scikit-learn\env\Scripts\python.exe
   machine: Windows-10-10.0.17763-SP0

BLAS:
    macros:
  lib_dirs:
cblas_libs: cblas

Python deps:
       pip: 18.1
setuptools: 40.6.3
   sklearn: 0.21.dev0
     numpy: 1.16.0
     scipy: 1.2.0
    Cython: 0.29.3
    pandas: None
```

Hints from issue discussion:
This may be a problem for #11886 given that SimpleImputer now handles
non-numerics

The issue is that `check_array` convert to `float64` object array. We need to turn `dtype=None` in case of `string` or `object` dtype to avoid the conversion.
> This may be a problem for #11886 given that SimpleImputer now handles non-numerics

It needs to be solved for the PR #12583 then.
If someone is going to work on this, I would like to add that for string types the error comes from the ```_get_mask``` function as discussed in #13028. This is in relation to numpy/numpy#5399
 
 I'm not sure that we want to support numpy string. 
                                                                                                                                      
 
                            
     
Sent from my phone - sorry to be brief and potential misspell. 

 


We should support whatever we support in SimpleImputer

Yes so no support for numpy string.

Commit message: TST: fix the error message match

Patch to review:
diff --git a/doc/whats_new/v0.20.rst b/doc/whats_new/v0.20.rst
index e9a1f831a7c32..21a466e433b64 100644
--- a/doc/whats_new/v0.20.rst
+++ b/doc/whats_new/v0.20.rst
@@ -36,6 +36,15 @@ Changelog
   threaded when `n_jobs > 1` or `n_jobs = -1`.
   :issue:`13005` by :user:`Prabakaran Kumaresshan <nixphix>`.
 
+:mod:`sklearn.impute`
+.....................
+
+- |Fix| add support for non-numeric data in
+  :class:`sklearn.impute.MissingIndicator` which was not supported while
+  :class:`sklearn.impute.SimpleImputer` was supporting these inputs for some
+  imputation strategies.
+  :issue:`13046` by :user:`Guillaume Lemaitre <glemaitre>`.
+
 :mod:`sklearn.linear_model`
 ...........................
 
diff --git a/sklearn/impute.py b/sklearn/impute.py
index 349af27eeb91e..e6a9278b47800 100644
--- a/sklearn/impute.py
+++ b/sklearn/impute.py
@@ -533,6 +533,23 @@ def _get_missing_features_info(self, X):
 
         return imputer_mask, features_with_missing
 
+    def _validate_input(self, X):
+        if not is_scalar_nan(self.missing_values):
+            force_all_finite = True
+        else:
+            force_all_finite = "allow-nan"
+        X = check_array(X, accept_sparse=('csc', 'csr'), dtype=None,
+                        force_all_finite=force_all_finite)
+        _check_inputs_dtype(X, self.missing_values)
+        if X.dtype.kind not in ("i", "u", "f", "O"):
+            raise ValueError("MissingIndicator does not support data with "
+                             "dtype {0}. 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.".format(X.dtype))
+        return X
+
     def fit(self, X, y=None):
         """Fit the transformer on X.
 
@@ -547,14 +564,7 @@ def fit(self, X, y=None):
         self : object
             Returns self.
         """
-        if not is_scalar_nan(self.missing_values):
-            force_all_finite = True
-        else:
-            force_all_finite = "allow-nan"
-        X = check_array(X, accept_sparse=('csc', 'csr'),
-                        force_all_finite=force_all_finite)
-        _check_inputs_dtype(X, self.missing_values)
-
+        X = self._validate_input(X)
         self._n_features = X.shape[1]
 
         if self.features not in ('missing-only', 'all'):
@@ -588,14 +598,7 @@ def transform(self, X):
 
         """
         check_is_fitted(self, "features_")
-
-        if not is_scalar_nan(self.missing_values):
-            force_all_finite = True
-        else:
-            force_all_finite = "allow-nan"
-        X = check_array(X, accept_sparse=('csc', 'csr'),
-                        force_all_finite=force_all_finite)
-        _check_inputs_dtype(X, self.missing_values)
+        X = self._validate_input(X)
 
         if X.shape[1] != self._n_features:
             raise ValueError("X has a different number of features "
diff --git a/sklearn/tests/test_impute.py b/sklearn/tests/test_impute.py
index 7131ac3ed0f5f..f68dbb8e28b30 100644
--- a/sklearn/tests/test_impute.py
+++ b/sklearn/tests/test_impute.py
@@ -13,6 +13,7 @@
 from sklearn.impute import MissingIndicator
 from sklearn.impute import SimpleImputer
 from sklearn.pipeline import Pipeline
+from sklearn.pipeline import make_union
 from sklearn.model_selection import GridSearchCV
 from sklearn import tree
 from sklearn.random_projection import sparse_random_matrix
@@ -55,7 +56,6 @@ def _check_statistics(X, X_true,
               err_msg=err_msg.format(True))
     assert_ae(X_trans, X_true, err_msg=err_msg.format(True))
 
-
 def test_imputation_shape():
     # Verify the shapes of the imputed matrix for different strategies.
     X = np.random.randn(10, 2)
@@ -509,7 +509,10 @@ def test_imputation_copy():
       "'features' has to be either 'missing-only' or 'all'"),
      (np.array([[-1, 1], [1, 2]]), np.array([[-1, 1], [1, 2]]),
       {'features': 'all', 'sparse': 'random'},
-      "'sparse' has to be a boolean or 'auto'")]
+      "'sparse' has to be a boolean or 'auto'"),
+     (np.array([['a', 'b'], ['c', 'a']], dtype=str),
+      np.array([['a', 'b'], ['c', 'a']], dtype=str),
+      {}, "MissingIndicator does not support data with dtype")]
 )
 def test_missing_indicator_error(X_fit, X_trans, params, msg_err):
     indicator = MissingIndicator(missing_values=-1)
@@ -614,6 +617,31 @@ def test_missing_indicator_sparse_param(arr_type, missing_values,
             assert isinstance(X_trans_mask, np.ndarray)
 
 
+def test_missing_indicator_string():
+    X = np.array([['a', 'b', 'c'], ['b', 'c', 'a']], dtype=object)
+    indicator = MissingIndicator(missing_values='a', features='all')
+    X_trans = indicator.fit_transform(X)
+    assert_array_equal(X_trans, np.array([[True, False, False],
+                                          [False, False, True]]))
+
+
+@pytest.mark.parametrize(
+    "X, missing_values, X_trans_exp",
+    [(np.array([['a', 'b'], ['b', 'a']], dtype=object), 'a',
+      np.array([['b', 'b', True, False], ['b', 'b', False, True]],
+               dtype=object)),
+     (np.array([[np.nan, 1.], [1., np.nan]]), np.nan,
+      np.array([[1., 1., True, False], [1., 1., False, True]]))]
+)
+def test_missing_indicator_with_imputer(X, missing_values, X_trans_exp):
+    trans = make_union(
+        SimpleImputer(missing_values=missing_values, strategy='most_frequent'),
+        MissingIndicator(missing_values=missing_values)
+    )
+    X_trans = trans.fit_transform(X)
+    assert_array_equal(X_trans, X_trans_exp)
+
+
 @pytest.mark.parametrize("imputer_constructor",
                          [SimpleImputer])
 @pytest.mark.parametrize(
diff --git a/sklearn/utils/estimator_checks.py b/sklearn/utils/estimator_checks.py
index 69850ecc5f796..77c557685aa13 100644
--- a/sklearn/utils/estimator_checks.py
+++ b/sklearn/utils/estimator_checks.py
@@ -74,10 +74,10 @@
                 'OrthogonalMatchingPursuit', 'PLSCanonical', 'PLSRegression',
                 'RANSACRegressor', 'RadiusNeighborsRegressor',
                 'RandomForestRegressor', 'Ridge', 'RidgeCV']
-
 ALLOW_NAN = ['Imputer', 'SimpleImputer', 'MissingIndicator',
              'MaxAbsScaler', 'MinMaxScaler', 'RobustScaler', 'StandardScaler',
              'PowerTransformer', 'QuantileTransformer']
+SUPPORT_STRING = ['SimpleImputer', 'MissingIndicator']
 
 
 def _yield_non_meta_checks(name, estimator):
@@ -625,9 +625,16 @@ def check_dtype_object(name, estimator_orig):
         if "Unknown label type" not in str(e):
             raise
 
-    X[0, 0] = {'foo': 'bar'}
-    msg = "argument must be a string or a number"
-    assert_raises_regex(TypeError, msg, estimator.fit, X, y)
+    if name not in SUPPORT_STRING:
+        X[0, 0] = {'foo': 'bar'}
+        msg = "argument must be a string or a number"
+        assert_raises_regex(TypeError, msg, estimator.fit, X, y)
+    else:
+        # Estimators supporting string will not call np.asarray to convert the
+        # data to numeric and therefore, the error will not be raised.
+        # Checking for each element dtype in the input array will be costly.
+        # Refer to #11401 for full discussion.
+        estimator.fit(X, y)
 
 
 def check_complex_data(name, estimator_orig):


Code review comments:
diff_hunk: @@ -36,6 +36,15 @@ Changelog
   threaded when `n_jobs > 1` or `n_jobs = -1`.
   :issue:`13005` by :user:`Prabakaran Kumaresshan <nixphix>`.
 
+:mod:`sklearn.impute`
+.....................
+
+- |Fix| add support for non-numeric data in
+  :class:`sklearn.impute.MissingIndicator` which was not supported while
+  :class:`sklearn.impute.SimpleImputer` was supporting these inputs for some
path: doc/whats_new/v0.20.rst
text: @user1:
```suggestion
  :class:`sklearn.impute.SimpleImputer` was supporting this for some
```