Repository: scikit-learn/scikit-learn
Pull request: [MRG] FIX: raise error with inconsistent dtype X and missing_values
closes #11390 

Implement an additional check to avoid raising `NotImplemented` from `np.equal` when the dtype of `X` and `missing_values` are inconsistent for a comparison.


Problem statement:
Cryptic error in imputers due to missing checking in _get_mask
By working on the `MissingIndicator` it seems that there is a missing checking between `X` and `missing_values` dtype:

```python
import numpy as np
X = np.array([[1.6464405 , 2.145568  , 1.80829   , 1.6346495 , 1.2709644 ],
              [1.3127615 , 2.675319  , 2.8906 , 2.1489816 , 0.8682183 ],
              [0.5495741 , 1.7595388 , 0.06032264, 2.4868202 , 0.01408643]],
             dtype=np.float32)
from sklearn.impute import SimpleImputer
trans = SimpleImputer(missing_values="NaN")
trans.fit_transform(X)
```

```
/home/lemaitre/Documents/code/toolbox/scikit-learn/sklearn/impute.py:59: FutureWarning: elementwise comparison failed; returning scalar instead, but in the future will perform elementwise comparison
  return np.equal(X, value_to_mask)
/home/lemaitre/Documents/code/toolbox/scikit-learn/sklearn/impute.py:59: FutureWarning: elementwise comparison failed; returning scalar instead, but in the future will perform elementwise comparison
  return np.equal(X, value_to_mask)
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
~/miniconda3/envs/dev/lib/python3.6/site-packages/numpy/core/fromnumeric.py in _wrapfunc(obj, method, *args, **kwds)
     51     try:
---> 52         return getattr(obj, method)(*args, **kwds)
     53 

TypeError: int() argument must be a string, a bytes-like object or a number, not 'NotImplementedType'

During handling of the above exception, another exception occurred:

TypeError                                 Traceback (most recent call last)
<ipython-input-18-151141bb4b39> in <module>()
----> 1 trans.fit_transform(X)

~/Documents/code/toolbox/scikit-learn/sklearn/base.py in fit_transform(self, X, y, **fit_params)
    457         if y is None:
    458             # fit method of arity 1 (unsupervised transformation)
--> 459             return self.fit(X, **fit_params).transform(X)
    460         else:
    461             # fit method of arity 2 (supervised transformation)

~/Documents/code/toolbox/scikit-learn/sklearn/impute.py in transform(self, X)
    417             mask = _get_mask(X, self.missing_values)
    418             n_missing = np.sum(mask, axis=0)
--> 419             values = np.repeat(valid_statistics, n_missing)
    420             coordinates = np.where(mask.transpose())[::-1]
    421 

~/miniconda3/envs/dev/lib/python3.6/site-packages/numpy/core/fromnumeric.py in repeat(a, repeats, axis)
    421 
    422     """
--> 423     return _wrapfunc(a, 'repeat', repeats, axis=axis)
    424 
    425 

~/miniconda3/envs/dev/lib/python3.6/site-packages/numpy/core/fromnumeric.py in _wrapfunc(obj, method, *args, **kwds)
     60     # a downstream library like 'pandas'.
     61     except (AttributeError, TypeError):
---> 62         return _wrapit(obj, method, *args, **kwds)
     63 
     64 

~/miniconda3/envs/dev/lib/python3.6/site-packages/numpy/core/fromnumeric.py in _wrapit(obj, method, *args, **kwds)
     40     except AttributeError:
     41         wrap = None
---> 42     result = getattr(asarray(obj), method)(*args, **kwds)
     43     if wrap:
     44         if not isinstance(result, mu.ndarray):

TypeError: int() argument must be a string, a bytes-like object or a number, not 'NotImplementedType'

```

In short, `NotImplement` is raised by equal in case of numeric and string mixed dtype. We should put a check in `_get_mask` which is shared across classes.

Hints from issue discussion:

Commit message: address jeremy comments

Patch to review:
diff --git a/sklearn/impute.py b/sklearn/impute.py
index 836bfb0167add..999689805b65d 100644
--- a/sklearn/impute.py
+++ b/sklearn/impute.py
@@ -40,6 +40,17 @@
 ]
 
 
+def _check_inputs_dtype(X, missing_values):
+    """Check that the dtype of X is in accordance with the one of
+    missing_values."""
+    if (X.dtype.kind in ("f", "i", "u") and
+            not isinstance(missing_values, numbers.Real)):
+        raise ValueError("The data type of 'missing_values' and 'X' are "
+                         "not compatible. 'missing_values' data type is "
+                         "{} and 'X' is {}."
+                         .format(type(missing_values), X.dtype))
+
+
 def _get_mask(X, value_to_mask):
     """Compute the boolean mask X == missing_values."""
     if value_to_mask is np.nan:
@@ -51,7 +62,6 @@ def _get_mask(X, value_to_mask):
         else:
             # np.isnan does not work on object dtypes.
             return _object_dtype_isnan(X)
-
     else:
         # X == value_to_mask with object dytpes does not always perform
         # element-wise for old versions of numpy
@@ -183,6 +193,7 @@ def _validate_input(self, X):
             else:
                 raise ve
 
+        _check_inputs_dtype(X, self.missing_values)
         if X.dtype.kind not in ("i", "u", "f", "O"):
             raise ValueError("SimpleImputer does not support data with dtype "
                              "{0}. Please provide either a numeric array (with"
@@ -788,6 +799,7 @@ def _initial_imputation(self, X):
 
         X = check_array(X, dtype=FLOAT_DTYPES, order="F",
                         force_all_finite=force_all_finite)
+        _check_inputs_dtype(X, self.missing_values)
 
         mask_missing_values = _get_mask(X, self.missing_values)
         if self.initial_imputer_ is None:
diff --git a/sklearn/tests/test_impute.py b/sklearn/tests/test_impute.py
index f5c42f7443487..906507cba93bd 100644
--- a/sklearn/tests/test_impute.py
+++ b/sklearn/tests/test_impute.py
@@ -705,3 +705,23 @@ def test_chained_imputer_additive_matrix():
                              random_state=rng).fit(X_train)
     X_test_est = imputer.transform(X_test)
     assert_allclose(X_test_filled, X_test_est, atol=0.01)
+
+
+@pytest.mark.parametrize("imputer_constructor",
+                         [SimpleImputer, ChainedImputer])
+@pytest.mark.parametrize(
+    "missing_values, X_missing_value, err_msg",
+    [("NaN", np.nan, "contains"),
+     ("-1", -1, "not compatible")])
+def test_inconsistent_dtype_X_missing_values(imputer_constructor,
+                                             missing_values, X_missing_value,
+                                             err_msg):
+    # regression test for issue #11390. Comparison between incoherent dtype
+    # for X and missing_values was not raising a proper error.
+    X = np.random.randn(10, 10)
+    X[0, 0] = X_missing_value
+
+    imputer = imputer_constructor(missing_values=missing_values)
+
+    with pytest.raises(ValueError, match=err_msg):
+        imputer.fit_transform(X)


Code review comments:
diff_hunk: @@ -705,3 +705,23 @@ def test_chained_imputer_additive_matrix():
                              random_state=rng).fit(X_train)
     X_test_est = imputer.transform(X_test)
     assert_allclose(X_test_filled, X_test_est, atol=0.01)
+
+
+@pytest.mark.parametrize("imputer_constructor",
+                         [SimpleImputer, ChainedImputer])
+@pytest.mark.parametrize(
+    "missing_values, X_missing_value, err_msg",
+    [("NaN", np.nan, "contains"),
path: sklearn/tests/test_impute.py
text: @user1:
Can you give more context for the contains error message? I don't know what the expected error is.

@user2:
It's the error raised by `check_array` when `X` contains `NaN` or `inf` and `force_all_finite=True:
```
Input contains NaN, infinity or a value too large for dtype('float64').
```

@user1:
right. would be easier to follow if it was "Input contains NaN" ...