Repository: pandas-dev/pandas
Pull request: BUG: bugfix 26390 assigning PandasArray to DataFrame error
- [x] closes #26390 
- [x] tests added / passed
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

----

Which section should i add the whatsnew entry in for this particular case?

Problem statement:
BUG: assigning Series.array / PandasArray to column fails
Assigning a PandasArray (so also the result of `df['a'].array`) of the correct length to add a column fails:

```python
In [1]: df = pd.DataFrame({'a': [1, 2, 3, 4], 'b': ['a', 'b', 'c', 'd']})                                                                                     

In [2]: df['c'] = pd.array([1, 2, None, 3])                                                                                                                   
---------------------------------------------------------------------------
KeyError                                  Traceback (most recent call last)
~/scipy/pandas/pandas/core/indexes/base.py in get_loc(self, key, method, tolerance)
   2672             try:
-> 2673                 return self._engine.get_loc(key)
   2674             except KeyError:

~/scipy/pandas/pandas/_libs/index.pyx in pandas._libs.index.IndexEngine.get_loc()

~/scipy/pandas/pandas/_libs/index.pyx in pandas._libs.index.IndexEngine.get_loc()

~/scipy/pandas/pandas/_libs/hashtable_class_helper.pxi in pandas._libs.hashtable.PyObjectHashTable.get_item()

~/scipy/pandas/pandas/_libs/hashtable_class_helper.pxi in pandas._libs.hashtable.PyObjectHashTable.get_item()

KeyError: 'c'

During handling of the above exception, another exception occurred:

KeyError                                  Traceback (most recent call last)
~/scipy/pandas/pandas/core/internals/managers.py in set(self, item, value)
   1048         try:
-> 1049             loc = self.items.get_loc(item)
   1050         except KeyError:

~/scipy/pandas/pandas/core/indexes/base.py in get_loc(self, key, method, tolerance)
   2674             except KeyError:
-> 2675                 return self._engine.get_loc(self._maybe_cast_indexer(key))
   2676         indexer = self.get_indexer([key], method=method, tolerance=tolerance)

~/scipy/pandas/pandas/_libs/index.pyx in pandas._libs.index.IndexEngine.get_loc()

~/scipy/pandas/pandas/_libs/index.pyx in pandas._libs.index.IndexEngine.get_loc()

~/scipy/pandas/pandas/_libs/hashtable_class_helper.pxi in pandas._libs.hashtable.PyObjectHashTable.get_item()

~/scipy/pandas/pandas/_libs/hashtable_class_helper.pxi in pandas._libs.hashtable.PyObjectHashTable.get_item()

KeyError: 'c'

During handling of the above exception, another exception occurred:

ValueError                                Traceback (most recent call last)
<ipython-input-2-03925b585d9b> in <module>
----> 1 df['c'] = pd.array([1, 2, None, 3])

~/scipy/pandas/pandas/core/frame.py in __setitem__(self, key, value)
   3334         else:
   3335             # set column
-> 3336             self._set_item(key, value)
   3337 
   3338     def _setitem_slice(self, key, value):

~/scipy/pandas/pandas/core/frame.py in _set_item(self, key, value)
   3410         self._ensure_valid_index(value)
   3411         value = self._sanitize_column(key, value)
-> 3412         NDFrame._set_item(self, key, value)
   3413 
   3414         # check if we are modifying a copy

~/scipy/pandas/pandas/core/generic.py in _set_item(self, key, value)
   3232 
   3233     def _set_item(self, key, value):
-> 3234         self._data.set(key, value)
   3235         self._clear_item_cache()
   3236 

~/scipy/pandas/pandas/core/internals/managers.py in set(self, item, value)
   1050         except KeyError:
   1051             # This item wasn't present, just insert at end
-> 1052             self.insert(len(self.items), item, value)
   1053             return
   1054 

~/scipy/pandas/pandas/core/internals/managers.py in insert(self, loc, item, value, allow_duplicates)
   1152 
   1153         block = make_block(values=value, ndim=self.ndim,
-> 1154                            placement=slice(loc, loc + 1))
   1155 
   1156         for blkno, count in _fast_count_smallints(self._blknos[loc:]):

~/scipy/pandas/pandas/core/internals/blocks.py in make_block(values, placement, klass, ndim, dtype, fastpath)
   3052         values = DatetimeArray._simple_new(values, dtype=dtype)
   3053 
-> 3054     return klass(values, ndim=ndim, placement=placement)
   3055 
   3056 

~/scipy/pandas/pandas/core/internals/blocks.py in __init__(self, values, placement, ndim)
   2584             values = np.array(values, dtype=object)
   2585 
-> 2586         super().__init__(values, ndim=ndim, placement=placement)
   2587 
   2588     @property

~/scipy/pandas/pandas/core/internals/blocks.py in __init__(self, values, placement, ndim)
     74 
     75     def __init__(self, values, placement, ndim=None):
---> 76         self.ndim = self._check_ndim(values, ndim)
     77         self.mgr_locs = placement
     78         self.values = values

~/scipy/pandas/pandas/core/internals/blocks.py in _check_ndim(self, values, ndim)
    111             msg = ("Wrong number of dimensions. values.ndim != ndim "
    112                    "[{} != {}]")
--> 113             raise ValueError(msg.format(values.ndim, ndim))
    114 
    115         return ndim

ValueError: Wrong number of dimensions. values.ndim != ndim [1 != 2]
```

Note this only fails for the PandasArray types (so when creating a FloatBlock or IntBlock, .. which expect 2D data, so when not creating an ExtensionBlock as is done for an "actual" ExtensionArray).

Hints from issue discussion:
This seems to work for me

```python
In [1]: import pandas as pd                                                     

In [2]: pd.__version__                                                          
Out[2]: '0.24.2'

In [3]: df = pd.DataFrame({'a': [1, 2, 3, 4], 'b': ['a', 'b', 'c', 'd']})       

In [4]: df['c'] = pd.array([1, 2, None, 3])                                     

In [5]: df                                                                      
Out[5]: 
   a  b     c
0  1  a     1
1  2  b     2
2  3  c  None
3  4  d     3
```
Indeed, on 0.24.2 it 'seems' to work, but we incorrectly store the PandasArray (which I think we fixed). It is however on master that this is failing.
The current master implementation does seem to convert any `PandasArray` to a a Numpy Array.

```python
if isinstance(values, ABCPandasArray):
        values = values.to_numpy()
if isinstance(dtype, PandasDtype):
        dtype = dtype.numpy_dtype
```
Thanks for looking into it! 
Yes, that's indeed what we want. But apparently something else still goes wrong. 
Without converting the PandasArray to a Numpy Array the block type assigned is `ExtensionBlock`. However, conversion to a NumpyArray results in the block type to be `ObjectBlock`.

`ExtensionBlock` is a child of `NonConsolidatableMixIn` which sets its `_validate_ndim` property to `False` hence when the `_check_ndim` check is performed no error is raised.

https://github.com/pandas-dev/pandas/blob/e5d15b20e828c66182a4e175dd5a5b9b951d9950/pandas/core/internals/blocks.py#L1511-L1515

This is not true for `ObjectBlock`, which has its `_validate_ndim` property set to `True`. Hence, the error is raised.

---

If we pass a NumpyArray instead of a PandasArray, then the during the call to the `_set_item` 
method of `NDFrame`, 

https://github.com/pandas-dev/pandas/blob/e5d15b20e828c66182a4e175dd5a5b9b951d9950/pandas/core/frame.py#L3400-L3413

the `_sanitize_column` method, when used with a numpy array explicitly converts it to 2-dimensions

https://github.com/pandas-dev/pandas/blob/e5d15b20e828c66182a4e175dd5a5b9b951d9950/pandas/core/frame.py#L3635

This step is left out when we convert the PandasArray to a numpy array. Perhaps we can add this after the `to_numpy()` conversion

https://github.com/pandas-dev/pandas/blob/e5d15b20e828c66182a4e175dd5a5b9b951d9950/pandas/core/internals/blocks.py#L3032-L3039

If we simply add a line `values = np.atleast_2d(np.asarray(values))` after Line 3036 in
https://github.com/pandas-dev/pandas/blob/e5d15b20e828c66182a4e175dd5a5b9b951d9950/pandas/core/internals/blocks.py#L3036

then the following test will fail

https://github.com/pandas-dev/pandas/blob/ff4437eb3b27440ea2304a8613369ebec0b8b72a/pandas/tests/internals/test_internals.py#L1295-L1312

However, with the bug in hand, and the new implementation of passing `PandasArray` by converting them to NumPy arrays, is this test valid now?

---

Another solution can be to convert the `PandasArray` to a `NumpyArray` during the `_sanitize_column` method, maybe here

https://github.com/pandas-dev/pandas/blob/e5d15b20e828c66182a4e175dd5a5b9b951d9950/pandas/core/frame.py#L3623-L3625

or add a special case of `ABCPandasArray`

Commit message: added release note message

Patch to review:
diff --git a/doc/source/whatsnew/v0.25.0.rst b/doc/source/whatsnew/v0.25.0.rst
index d38ee7b8b589a..70e8a6baad337 100644
--- a/doc/source/whatsnew/v0.25.0.rst
+++ b/doc/source/whatsnew/v0.25.0.rst
@@ -475,7 +475,7 @@ Other
 - Removed unused C functions from vendored UltraJSON implementation (:issue:`26198`)
 - Bug in :func:`factorize` when passing an ``ExtensionArray`` with a custom ``na_sentinel`` (:issue:`25696`).
 - Allow :class:`Index` and :class:`RangeIndex` to be passed to numpy ``min`` and ``max`` functions.
-
+- Fixed bug where assigning a :class:`pandas.core.arrays.numpy_.PandasArray` to a :class:`pandas.core.frame.DataFrame` would raise error (:issue:`26390`)
 
 .. _whatsnew_0.250.contributors:
 
diff --git a/pandas/core/internals/blocks.py b/pandas/core/internals/blocks.py
index 0c49ebb55acdd..8e47467538dd4 100644
--- a/pandas/core/internals/blocks.py
+++ b/pandas/core/internals/blocks.py
@@ -3035,6 +3035,9 @@ def make_block(values, placement, klass=None, ndim=None, dtype=None,
     # For now, blocks should be backed by ndarrays when possible.
     if isinstance(values, ABCPandasArray):
         values = values.to_numpy()
+        if ndim and ndim > 1:
+            values = np.atleast_2d(values)
+
     if isinstance(dtype, PandasDtype):
         dtype = dtype.numpy_dtype
 
diff --git a/pandas/tests/internals/test_internals.py b/pandas/tests/internals/test_internals.py
index fbd821f8ec342..479b01571f909 100644
--- a/pandas/tests/internals/test_internals.py
+++ b/pandas/tests/internals/test_internals.py
@@ -17,7 +17,8 @@
     SparseArray)
 import pandas.core.algorithms as algos
 from pandas.core.arrays import DatetimeArray, TimedeltaArray
-from pandas.core.internals import BlockManager, SingleBlockManager, make_block
+from pandas.core.internals import (
+    BlockManager, ObjectBlock, SingleBlockManager, make_block)
 import pandas.util.testing as tm
 from pandas.util.testing import (
     assert_almost_equal, assert_frame_equal, assert_series_equal, randn)
@@ -1310,3 +1311,14 @@ def test_make_block_no_pandas_array():
     result = make_block(arr.to_numpy(), slice(len(arr)), dtype=arr.dtype)
     assert result.is_integer is True
     assert result.is_extension is False
+
+
+def test_add_column_with_pandas_array():
+    # GH 26390
+    df = pd.DataFrame({'a': [1, 2, 3, 4], 'b': ['a', 'b', 'c', 'd']})
+    df['c'] = pd.array([1, 2, None, 3])
+    df2 = pd.DataFrame({'a': [1, 2, 3, 4], 'b': ['a', 'b', 'c', 'd'],
+                        'c': pd.array([1, 2, None, 3])})
+    assert(df2['c']._data.blocks[0].__class__ == ObjectBlock)
+    assert(df['c']._data.blocks[0].__class__ == ObjectBlock)
+    assert_frame_equal(df, df2)


Code review comments:
diff_hunk: @@ -475,7 +475,7 @@ Other
 - Removed unused C functions from vendored UltraJSON implementation (:issue:`26198`)
 - Bug in :func:`factorize` when passing an ``ExtensionArray`` with a custom ``na_sentinel`` (:issue:`25696`).
 - Allow :class:`Index` and :class:`RangeIndex` to be passed to numpy ``min`` and ``max`` functions.
-
+- Fixed bug where assigning a :class:`pandas.core.arrays.numpy_.PandasArray` to a :class:`pandas.core.frame.DataFrame` would raise error (:issue:`26390`)
path: doc/source/whatsnew/v0.25.0.rst
text: @user1:
should be :class:\`arrays.PandasArray\`

@user2:
move to the indexing section

@author:
sure will do so in the next commit

diff_hunk: @@ -1310,3 +1311,14 @@ def test_make_block_no_pandas_array():
     result = make_block(arr.to_numpy(), slice(len(arr)), dtype=arr.dtype)
     assert result.is_integer is True
     assert result.is_extension is False
+
+
+def test_add_column_with_pandas_array():
+    # GH 26390
+    df = pd.DataFrame({'a': [1, 2, 3, 4], 'b': ['a', 'b', 'c', 'd']})
path: pandas/tests/internals/test_internals.py
text: @user1:
this test doesn't belong here, put instead in pandas/tests/frame/test_block_internals.py

@author:
sure. doing that in the next commit