Repository: scikit-learn/scikit-learn
Pull request: [MRG] Convert ColumnTransformer input list to numpy array
<!--
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/master/CONTRIBUTING.md#pull-request-checklist
-->

#### Reference Issues/PRs
<!--
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
-->
Fixes #12096.

#### What does this implement/fix? Explain your changes.
Converts the input list for ColumnTransformer to a numpy array.

Added a check inside `transform` and `fit_transform` to check if the input `X` is a list, if it is then it gets converted to a numpy array.

#### Any other comments?
Should this conversion be documented in the docstrings for ColumnTransfomer's `fit`, `transform` and `fit_transform`?

<!--
Please be aware that we are a loose team of volunteers so patience is
necessary; assistance handling other issues is very welcome. We value
all user contributions, no matter how minor they are. If we are slow to
review, either the pull request needs some benchmarking, tinkering,
convincing, etc. or more likely the reviewers are simply busy. In either
case, we ask for your understanding during the review process.
For more information, see our FAQ on this topic:
http://scikit-learn.org/dev/faq.html#why-is-my-pull-request-not-getting-any-attention.

Thanks for contributing!
-->


Problem statement:
ColumnTransformer breaks where X is a list
```py
>>> from sklearn.preprocessing import StandardScaler
>>> from sklearn.compose import ColumnTransformer
>>> ColumnTransformer([('foobar', StandardScaler(), [0, 1, 2])]).fit([[1, 2, 3]])
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/joel/repos/scikit-learn/sklearn/compose/_column_transformer.py", line 398, in fit
    self.fit_transform(X, y=y)
  File "/Users/joel/repos/scikit-learn/sklearn/compose/_column_transformer.py", line 422, in fit_transform
    self._validate_remainder(X)
  File "/Users/joel/repos/scikit-learn/sklearn/compose/_column_transformer.py", line 275, in _validate_remainder
    n_columns = X.shape[1]
AttributeError: 'list' object has no attribute 'shape'
```

The passed list should be interpreted as an array for the sake of extracting columns. Instead an error is raised.

Hints from issue discussion:

Commit message: Add check_array

Patch to review:
diff --git a/sklearn/compose/_column_transformer.py b/sklearn/compose/_column_transformer.py
index e09d2d09d7e43..932e32b988983 100644
--- a/sklearn/compose/_column_transformer.py
+++ b/sklearn/compose/_column_transformer.py
@@ -21,7 +21,7 @@
 from ..preprocessing import FunctionTransformer
 from ..utils import Bunch
 from ..utils.metaestimators import _BaseComposition
-from ..utils.validation import check_is_fitted
+from ..utils.validation import check_array, check_is_fitted
 
 
 __all__ = ['ColumnTransformer', 'make_column_transformer']
@@ -419,6 +419,8 @@ def fit_transform(self, X, y=None):
             sparse matrices.
 
         """
+        X = check_array(X)
+
         self._validate_remainder(X)
         self._validate_transformers()
 
@@ -467,6 +469,8 @@ def transform(self, X):
         """
         check_is_fitted(self, 'transformers_')
 
+        X = check_array(X)
+
         Xs = self._fit_transform(X, None, _transform_one, fitted=True)
         self._validate_output(Xs)
 
diff --git a/sklearn/compose/tests/test_column_transformer.py b/sklearn/compose/tests/test_column_transformer.py
index f67806a52c543..4cb7ba8ab6f4b 100644
--- a/sklearn/compose/tests/test_column_transformer.py
+++ b/sklearn/compose/tests/test_column_transformer.py
@@ -278,6 +278,16 @@ def test_column_transformer_sparse_array():
                                      X_res_both)
 
 
+def test_column_transformer_list():
+    X_list = [[1, 2, 3]]
+    X_res = np.array([[0, 0, 0]])
+
+    ct = ColumnTransformer([('trans1', StandardScaler(), [0, 1, 2])])
+
+    assert_array_equal(ct.fit_transform(X_list), X_res)
+    assert_array_equal(ct.fit(X_list).transform(X_list), X_res)
+
+
 def test_column_transformer_sparse_stacking():
     X_array = np.array([[0, 1, 2], [2, 4, 6]]).T
     col_trans = ColumnTransformer([('trans1', Trans(), [0]),


Code review comments:
diff_hunk: @@ -467,6 +469,8 @@ def transform(self, X):
         """
         check_is_fitted(self, 'transformers_')
 
+        X = check_array(X)
path: sklearn/compose/_column_transformer.py
text: @user1:
A naive use of check_array will break pandas support. I think we should do something like:

```python
if not hasattr(X, 'loc'):
    X = check_array(X)
```

We don't want to do an explicit `isinstance` to avoid introducing an explicit dependency on the pandas library.

@user2:
SShouldn't such a check be done in `check_array`? Or else we should have this check almost everywhere if we want to keep the dataframe, don't we?

@user3:
Further, `check_array` should also not convert to numeric or raise on sparse in this case (it should basically do no check at all). Therefore, in this case, I would maybe not use check_array?

I think in the end we want to improve `check_array` to also handle pandas dataframes, but that is a larger endeavour than fixing this bug. 
@user2 the column transformer is currently the only sklearn object where we don't convert the passed dataframe to an array, therefore this issue does not come up everywhere else

@user3:
One possibility would be to only call `check_array` if we actually have a list (or the other way around if X is not yet a numpy ndarray or pandas dataframe).

Further, it should also not enforce all finite values in this case.

@user4:
> One possibility would be to only call check_array if we actually have a list (or the other way around if X is not yet a numpy ndarray or pandas dataframe).

So this is similar to ogrisel's solution above? I'm +1 to that solution (Maybe also add a comment to note down the limitation of current check_array).

@user3:
Yes, with the difference that we also should not call check_array for arrays (Olivier only checks for dataframe), or otherwise specify a lot of flags to check_array to accept sparse, not error on nan data, ..

@author:
> One possibility would be to only call check_array if we actually have a list (or the other way around if X is not yet a numpy ndarray or pandas dataframe).

> Yes, with the difference that we also should not call check_array for arrays (Olivier only checks for dataframe), or otherwise specify a lot of flags to check_array to accept sparse, not error on nan data, ..

So for this bug fix, we should just check if X is a `list` using `isinstance` and then use `check_array` to convert it to an array, right?

@user5:
Perhaps

```py
if not hasattr(X, '__array__'):
    X = np.asarray(X)
```
?

(or do we want dtype='object' to be safe?)

@user3:
We want dtype='object' arrays to not be converted I think, but ndarrays do have a `__array__`, so your proposal is fine for that?

@user3:
Ah, sparse matrices apparently have no `__array__`, so an additional check should include that as well

@user3:
> Shouldn't such a check be done in check_array?

Opened a new issue about this here: https://github.com/scikit-learn/scikit-learn/issues/12148