Skip to content

BUG: bugfix 26390 assigning PandasArray to DataFrame error #26417

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
May 19, 2019
Merged

BUG: bugfix 26390 assigning PandasArray to DataFrame error #26417

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
fcd29ed
bugfix 26390 assigning pandas array to column works
shantanu-gontia May 15, 2019
a4f6e89
put conversion to ndarray in make_block
shantanu-gontia May 15, 2019
b3da29f
fixed imports
shantanu-gontia May 15, 2019
c44c6e3
Merge remote-tracking branch 'upstream/master' into bug26390
shantanu-gontia May 16, 2019
8a4c87f
added release note message
shantanu-gontia May 16, 2019
cd1d38b
changed test location to block_internals
shantanu-gontia May 19, 2019
f1e3098
changed section of release note
shantanu-gontia May 19, 2019
997c0a5
fixed import sort order
shantanu-gontia May 19, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion doc/source/whatsnew/v0.25.0.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:

Expand Down
3 changes: 3 additions & 0 deletions pandas/core/internals/blocks.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
14 changes: 13 additions & 1 deletion pandas/tests/internals/test_internals.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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


Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think you actually need this test; rather in the test right above, check the result.values is is the correct type

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this test was already present, was this valid only prior to the decision of converting PandasArray to numpy array?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that wouuld quite hit the issue. I think you would need to add a new result = make_black(arr, ..., ndim=2) with the right placement. It wouldn't hurt to add that, but I think keep the test below.

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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We want assert statements

Suggested change
assert(df2['c']._data.blocks[0].__class__ == ObjectBlock)
assert type(df2['c']._data.blocks[0]) == ObjectBlock

Same for the line below.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you remove these parens?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure, will do so in the next commit

assert(df['c']._data.blocks[0].__class__ == ObjectBlock)
assert_frame_equal(df, df2)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you assert that df2['c']._data.blocks[0] is an ObjectBlock (not an extension block).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will add these checks