style: end with "."

Actually this should be:

if isinstance(self.missing_value, numbers.Real) and np.isnan(self.missing_values):
    force_all_finite = "allow-nan"
else:
    force_all_finite = True

or even:

force_all_finite = True if self.missing_values is not np.nan else 'allow-nan'

is -> if

I would add that this way it also works for object dtypes (in case somebody would wonder why not np.isnan is used)

Sphinx does not really like this way of wrapping the line ..
If you want it to render nicely we can do

    missing_values : real number, string, np.nan or None, \
optional (default=np.nan).
         The placeholder for ...

nitpick: comment fits on one line.

This comment has been copied several times.

Maybe this should always be:

return np.full(X.shape[1], fill_value, dtype=X.dtype)

independently of fill_value no? If fill_value is inconsistent with X.dtype after validation, fit should have already raised an informative error message above.

This should probably be a TypeError because we check isintance in the condition.

We should also issue informative error messages the following cases:

elif X.dtype.kind == "O" and fill_value not in six.string_types:
        raise TypeError("fill_value={0} is invalid. Expected the an str instance when" 
                        " imputing categorical data.".format(fill_value))
else:
    raise ValueError("SimpleImputer cannot work on data with dtype={0}:"
                     " expecting numerical or categorical data with dtype=object"
                     "".format(X.dtype))

We should also add tests to check that those exceptions and their message are raised on invalid inputs.

git grep "with pytest.raises" to find examples on how to test exception in the scikit-learn test suite using pytest.

style consistency: no need for a blank line here.

style:please define a local variable to improve readability:

force_all_finite = "allow-nan" if self.missing_values is np.nan else True

Maybe this should be:

most_frequent = np.empty(X.shape[0], dtype=X.dtype)

@ogrisel actually, we don't always want most_frequent to have the same dtype as X. For example, if there is a column full of NaNs in an integer array, X has integer dtype, whereas most_frequent (which stores the statistics column-wise) will have a np.nan for the column of NaNs.

so discussed here: if you have integer with all -1, specify missing_value of -1, then the most_frequent will be NaN (due to implementation details to signal this case), and then X.dtype would clash with NaN.

ok

I am not sure it is good / needed to be that strict here. For example, you can have Categorical data in pandas with integer categories (or other non-string values) where it can make sense to fill with another value (and column.dtype.kind will be "O" for categorical columns)

We should have a test for this specific use case: imputing a categorical column with a pandas categorical value.

maybe
if not (isinstance(fill_value, six.string_types) or isinstance(fill_value, numbers.Reals) or fill_value is None)
is enough ?

We talked IRL with @jorisvandenbossche and if the categorical datafram has integer values with missing values, then the check_array conversion will yield a floating point array so its already covered by the existing tests.

Let's stay strict in this checks for now. If a user reports a valid use case for being laxer we can always change that later.

No test coverage

Not tested

Please replace these two lines with:

    missing_values : number, string, np.nan (default) or None

what we currently have here is unnecessarily verbose.

Hmmm... object dtype does not necessarily mean string data. And string data does not necessarily mean object dtype (numpy.asarray with strings will not result in object data; we need to test this case). So I'm not sure this validation or its error message is quite right. I don't think we need to support numeric data in object arrays necessarily, but we do need to support string arrays if we're going to support object arrays, IMO, and we need to make sure the error messages are appropriate to the various cases.

+1 for adding support and tests for accepting string arrays even if in most cases that are not a recommended data structure for representing categorical variables.

As for the input validation on object dtype array, we can just drop the elif X.dtype.kind == "O" case and not raise any TypeError and accept any fill_value when X.dtype.kind == "O".

Note that there are some complexities to deal with in that case, eg the fact that filling with new values might truncate silently:

In [5]: a = np.array(['a', 'b', 'a'])

In [6]: mask = a == 'b'

In [7]: a[mask] = 'missing'

In [8]: a
Out[8]: 
array(['a', 'm', 'a'],
      dtype='<U1')

But that's then up to the user's responsibility?

object dtype does not necessarily mean string data

On this part (so not about supporting string dtype), I think we briefly discussed this and our feeling was: "let's be more restrictive than theoretically needed for now, if there is request for it we can always relax later".

For example, a situation where you can get the case of array of object dtype consisting of mixed types (and not only strings), is if you pass a subset of columns of a dataframe with both string and numerical columns to the SimpleImputer. But, normally you will want a different fill value for both types of columns, and you will need a ColumnTransformer anyhow to have a specific SimpleImputer for each type of columns.

Can't we force dtype = object as soon as input dtype is not numeric ?
Something like:

if X.dtype.kind not in ("i", "f"):
    X = X.astype(object)

right after check_array ?

Thanks @jorisvandenbossche, indeed imputing a string array is not simple to implement and probably useless in practice.

I think I prefer to raise an informative error message asking the user to provide either numerical data with integer or floating point data types or categorical represented with integer or object datatypes.

If we discover a common pipeline where the implicit conversion from string dtype to object dtype suggested above by @jeremiedbb is helpful we might want to consider it later, but to me it sounds like a YAGNI.

Please note here (or in a Notes section) that most_frequent and constant work with strings or numeric data, while the others only work with numeric data.

I am wondering: do we usually raise TypeError or ValueError when the object type is valid, but the dtype is not?

Also, the message is a bit confusing: one could get the impression that float data is not supported.

"""
SimpleImputer does not work on 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.
"""

Better be explicit in error message. Verbosity is not a problem here.

numpy raises TypeError for bad dtypes. E.g. np.log(np.asarray(['hello']))

numpy is not very consistent:

>>> np.array(['a', 'b'], dtype=object).astype(np.uint8)
Traceback (most recent call last):
  File "<ipython-input-6-3200a6709d46>", line 1, in <module>
    np.array(['a', 'b'], dtype=object).astype(np.uint8)
ValueError: invalid literal for int() with base 10: 'a'

It's true that if you consider the individual scalar operation the TypeError raised by a ufunc might make sense. If you consider that the array as a whole is invalid, then ValueError makes more sense. I think both are fine in this case.

As we switched to ValueError above we should probably be consistent and do it here as well.

I think value error is fine because this is already what is raised by most models who use the standard check_X_y validation:

>>> from sklearn.linear_model import LogisticRegression
>>> LogisticRegression().fit([['invalid'], ['invalid']], [0, 1])
Traceback (most recent call last):
  File "<ipython-input-6-91496e8d832f>", line 1, in <module>
    LogisticRegression().fit([['invalid', 'invalid']], [0, 1])
  File "/home/ogrisel/code/scikit-learn/sklearn/linear_model/logistic.py", line 1217, in fit
    order="C")
  File "/home/ogrisel/code/scikit-learn/sklearn/utils/validation.py", line 671, in check_X_y
    ensure_min_features, warn_on_dtype, estimator)
  File "/home/ogrisel/code/scikit-learn/sklearn/utils/validation.py", line 494, in check_array
    array = np.asarray(array, dtype=dtype, order=order)
  File "/home/ogrisel/.virtualenvs/py36/lib/python3.6/site-packages/numpy/core/numeric.py", line 492, in asarray
    return array(a, dtype, copy=False, order=order)
ValueError: could not convert string to float: 'invalid'