Skip to content

BUG: enable test_numpy tests with ArrayManager #42780

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
Aug 5, 2021
Merged

BUG: enable test_numpy tests with ArrayManager #42780

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
79fd984
BUG: enable test_numpy tests with ArrayManager
jbrockmendel Jul 28, 2021
3e2f66f
Merge branch 'master' into bug-am-pandasarray
jbrockmendel Jul 30, 2021
a0eb54f
Merge branch 'master' into bug-am-pandasarray
jbrockmendel Jul 30, 2021
d6f331f
Merge branch 'master' into bug-am-pandasarray
jbrockmendel Jul 31, 2021
8d49d61
Merge branch 'master' into bug-am-pandasarray
jbrockmendel Aug 1, 2021
5366a26
catch StringArray
jbrockmendel Aug 2, 2021
c3b55d2
Merge branch 'master' into bug-am-pandasarray
jbrockmendel Aug 3, 2021
912b044
troubleshoot
jbrockmendel Aug 3, 2021
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
4 changes: 4 additions & 0 deletions pandas/core/indexing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1697,6 +1697,10 @@ def _setitem_with_indexer_split_path(self, indexer, value, name: str):
# We get here in one case via .loc with a all-False mask
pass

elif self._is_scalar_access(indexer):
# We are setting nested data
self._setitem_single_column(indexer[1], value, pi)

elif len(ilocs) == len(value):
# We are setting multiple columns in a single row.
for loc, v in zip(ilocs, value):
Expand Down
8 changes: 5 additions & 3 deletions pandas/core/internals/array_manager.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@
)
from pandas.core.dtypes.generic import (
ABCDataFrame,
ABCPandasArray,
ABCSeries,
)
from pandas.core.dtypes.inference import is_inferred_bool_dtype
Expand Down Expand Up @@ -85,6 +84,7 @@
from pandas.core.internals.blocks import (
ensure_block_shape,
external_values,
extract_pandas_array,
maybe_coerce_values,
new_block,
to_native_types,
Expand Down Expand Up @@ -399,6 +399,8 @@ def convert(
) -> T:
def _convert(arr):
if is_object_dtype(arr.dtype):
# extract PandasArray for tests that patch PandasArray._typ
arr = np.asarray(arr)
return soft_convert_objects(
arr,
datetime=datetime,
Expand Down Expand Up @@ -697,6 +699,7 @@ def __init__(

if verify_integrity:
self._axes = [ensure_index(ax) for ax in axes]
arrays = [extract_pandas_array(x, None, 1)[0] for x in arrays]
Copy link
Member

Choose a reason for hiding this comment

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

Slightly off topic (since we already do this elsewhere), but this check is only needed for when using the patched version in the tests?
If so, I am wondering if we should use some "TEST_MODE" env variable so we can check that here and do this step only when running the patched test, to avoid the overhead this is adding for real code.

Copy link
Member Author

Choose a reason for hiding this comment

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

yah i increasingly think that test file is more trouble than its worth. we should try to salvage the subset of tests that directly test PandasArray and disable the others

self.arrays = [maybe_coerce_values(arr) for arr in arrays]
self._verify_integrity()

Expand Down Expand Up @@ -1183,8 +1186,7 @@ def __init__(
self._axes = [ensure_index(ax) for ax in self._axes]
arr = arrays[0]
arr = maybe_coerce_values(arr)
if isinstance(arr, ABCPandasArray):
arr = arr.to_numpy()
arr = extract_pandas_array(arr, None, 1)[0]
self.arrays = [arr]
self._verify_integrity()

Expand Down
25 changes: 0 additions & 25 deletions pandas/tests/extension/test_numpy.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@
import numpy as np
import pytest

import pandas.util._test_decorators as td

from pandas.core.dtypes.cast import can_hold_element
from pandas.core.dtypes.dtypes import (
ExtensionDtype,
Expand All @@ -30,9 +28,6 @@
from pandas.core.internals import blocks
from pandas.tests.extension import base

# TODO(ArrayManager) PandasArray
pytestmark = td.skip_array_manager_not_yet_implemented


def _can_hold_element_patched(obj, element) -> bool:
if isinstance(element, PandasArray):
Expand Down Expand Up @@ -349,26 +344,6 @@ def test_setitem_sequence_broadcasts(self, data, box_in_series):
# length than the value
super().test_setitem_sequence_broadcasts(data, box_in_series)

@skip_nested
def test_setitem_loc_scalar_mixed(self, data):
# AssertionError
super().test_setitem_loc_scalar_mixed(data)
Copy link
Member

Choose a reason for hiding this comment

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

Where those already working? (as those are generally skipped, not only for ArrayManager)

Copy link
Member Author

Choose a reason for hiding this comment

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

AFAICT the added check in indexing.py fixed this for both AM and BM.

Copy link
Contributor

Choose a reason for hiding this comment

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

do we need a note for this? e.g. is this a user facing bug fix?

Copy link
Member Author

Choose a reason for hiding this comment

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

no and no


@skip_nested
def test_setitem_loc_scalar_multiple_homogoneous(self, data):
# AssertionError
super().test_setitem_loc_scalar_multiple_homogoneous(data)

@skip_nested
def test_setitem_iloc_scalar_mixed(self, data):
# AssertionError
super().test_setitem_iloc_scalar_mixed(data)

@skip_nested
def test_setitem_iloc_scalar_multiple_homogoneous(self, data):
# AssertionError
super().test_setitem_iloc_scalar_multiple_homogoneous(data)

@skip_nested
@pytest.mark.parametrize("setter", ["loc", None])
def test_setitem_mask_broadcast(self, data, setter):
Expand Down