-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
TST: expand tests for ExtensionArray setitem with nullable arrays #31741
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.
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
TomAugspurger
merged 9 commits into
pandas-dev:master
from
jorisvandenbossche:setitem-tests
Feb 13, 2020
Merged
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
a62dbda
TST: expand tests for ExtensionArray setitem with nullable arrays
jorisvandenbossche 3d713ef
combine tests
jorisvandenbossche d2e1af5
fix numpy test
jorisvandenbossche af11ed0
test msg
jorisvandenbossche ceb1a3c
Merge remote-tracking branch 'upstream/master' into setitem-tests
jorisvandenbossche 8796c6e
use xfail
jorisvandenbossche 0be92b7
fix numpy test
jorisvandenbossche ac970c6
Merge remote-tracking branch 'upstream/master' into setitem-tests
jorisvandenbossche db73fff
add xfail reason
jorisvandenbossche File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,7 +4,7 @@ | |
import pytest | ||
|
||
import pandas as pd | ||
from pandas.core.arrays.numpy_ import PandasDtype | ||
import pandas._testing as tm | ||
|
||
from .base import BaseExtensionTests | ||
|
||
|
@@ -93,6 +93,92 @@ def test_setitem_iloc_scalar_multiple_homogoneous(self, data): | |
df.iloc[10, 1] = data[1] | ||
assert df.loc[10, "B"] == data[1] | ||
|
||
@pytest.mark.parametrize( | ||
"mask", | ||
[ | ||
np.array([True, True, True, False, False]), | ||
pd.array([True, True, True, False, False], dtype="boolean"), | ||
jreback marked this conversation as resolved.
Show resolved
Hide resolved
|
||
], | ||
ids=["numpy-array", "boolean-array"], | ||
jreback marked this conversation as resolved.
Show resolved
Hide resolved
|
||
) | ||
def test_setitem_mask(self, data, mask, box_in_series): | ||
arr = data[:5].copy() | ||
expected = arr.take([0, 0, 0, 3, 4]) | ||
if box_in_series: | ||
arr = pd.Series(arr) | ||
expected = pd.Series(expected) | ||
arr[mask] = data[0] | ||
self.assert_equal(expected, arr) | ||
jorisvandenbossche marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
def test_setitem_mask_raises(self, data, box_in_series): | ||
# wrong length | ||
mask = np.array([True, False]) | ||
|
||
if box_in_series: | ||
data = pd.Series(data) | ||
|
||
with pytest.raises(IndexError, match="wrong length"): | ||
data[mask] = data[0] | ||
|
||
mask = pd.array(mask, dtype="boolean") | ||
with pytest.raises(IndexError, match="wrong length"): | ||
data[mask] = data[0] | ||
|
||
def test_setitem_mask_boolean_array_raises(self, data, box_in_series): | ||
# missing values in mask | ||
mask = pd.array(np.zeros(data.shape, dtype="bool"), dtype="boolean") | ||
mask[:2] = pd.NA | ||
|
||
if box_in_series: | ||
data = pd.Series(data) | ||
|
||
msg = ( | ||
"Cannot mask with a boolean indexer containing NA values|" | ||
"cannot mask with array containing NA / NaN values" | ||
) | ||
with pytest.raises(ValueError, match=msg): | ||
data[mask] = data[0] | ||
|
||
@pytest.mark.parametrize( | ||
"idx", | ||
[[0, 1, 2], pd.array([0, 1, 2], dtype="Int64"), np.array([0, 1, 2])], | ||
ids=["list", "integer-array", "numpy-array"], | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. labels should be: extension-integer-array, numpy-integer-array to be more informative |
||
) | ||
def test_setitem_integer_array(self, data, idx, box_in_series): | ||
arr = data[:5].copy() | ||
expected = data.take([0, 0, 0, 3, 4]) | ||
|
||
if box_in_series: | ||
arr = pd.Series(arr) | ||
expected = pd.Series(expected) | ||
|
||
arr[idx] = arr[0] | ||
self.assert_equal(arr, expected) | ||
|
||
@pytest.mark.parametrize( | ||
"idx, box_in_series", | ||
[ | ||
([0, 1, 2, pd.NA], False), | ||
pytest.param( | ||
[0, 1, 2, pd.NA], True, marks=pytest.mark.xfail(reason="GH-31948") | ||
), | ||
(pd.array([0, 1, 2, pd.NA], dtype="Int64"), False), | ||
(pd.array([0, 1, 2, pd.NA], dtype="Int64"), False), | ||
], | ||
ids=["list-False", "list-True", "integer-array-False", "integer-array-True"], | ||
) | ||
def test_setitem_integer_with_missing_raises(self, data, idx, box_in_series): | ||
arr = data.copy() | ||
|
||
# TODO(xfail) this raises KeyError about labels not found (it tries label-based) | ||
# for list of labels with Series | ||
if box_in_series: | ||
arr = pd.Series(data, index=[tm.rands(4) for _ in range(len(data))]) | ||
|
||
msg = "Cannot index with an integer indexer containing NA values" | ||
with pytest.raises(ValueError, match=msg): | ||
arr[idx] = arr[0] | ||
|
||
@pytest.mark.parametrize("as_callable", [True, False]) | ||
@pytest.mark.parametrize("setter", ["loc", None]) | ||
def test_setitem_mask_aligned(self, data, as_callable, setter): | ||
|
@@ -219,14 +305,3 @@ def test_setitem_preserves_views(self, data): | |
data[0] = data[1] | ||
assert view1[0] == data[1] | ||
assert view2[0] == data[1] | ||
|
||
def test_setitem_nullable_mask(self, data): | ||
# GH 31446 | ||
# TODO: there is some issue with PandasArray, therefore, | ||
# TODO: skip the setitem test for now, and fix it later | ||
if data.dtype != PandasDtype("object"): | ||
arr = data[:5] | ||
expected = data.take([0, 0, 0, 3, 4]) | ||
mask = pd.array([True, True, True, False, False]) | ||
arr[mask] = data[0] | ||
self.assert_extension_array_equal(expected, arr) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
think about adding a new class here; these are specifically for setitem_mask (could be a followon)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general, changing names is annoying for downstream consumers of those tests (this are the base extension tests), so not worth here I think
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, I don't buy your argument, these are experimental features where we actually to expand test coverage. its going to happen (but ok for now)