Skip to content

CLN/TST: delegate StringArray.fillna() to parent class + add tests #37987

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 19 commits into from
Nov 26, 2020
Merged

CLN/TST: delegate StringArray.fillna() to parent class + add tests #37987

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
e6a3f40
ENH: add dtype validation for value in StringArray.fillna
arw2019 Nov 21, 2020
0a85704
TST: test fillna args
arw2019 Nov 21, 2020
4814614
ENH: add dtype validation for value in StringArray.fillna
arw2019 Nov 21, 2020
7051e97
TST: test fillna args
arw2019 Nov 21, 2020
df8e185
feedback: broaden validation to np.str_ + allow NA fill value
arw2019 Nov 21, 2020
43a6646
TST: add tests with np._str, missing value
arw2019 Nov 21, 2020
616a6c7
skip check if no value provided
arw2019 Nov 21, 2020
a43de61
feedback: remove np.str_ check
arw2019 Nov 21, 2020
f6eff15
Merge branch 'master' of https://github.com/pandas-dev/pandas into st…
arw2019 Nov 21, 2020
24cb928
add is_string_dtype check
arw2019 Nov 21, 2020
9b2f918
merge master
arw2019 Nov 23, 2020
a014784
merge origin
arw2019 Nov 23, 2020
02ff905
Merge branch 'master' of https://github.com/pandas-dev/pandas into st…
arw2019 Nov 24, 2020
b658791
rewrite using _validate_setitem_value
arw2019 Nov 24, 2020
135f55a
revert changes to base class
arw2019 Nov 24, 2020
ef1f800
Merge branch 'master' of https://github.com/pandas-dev/pandas into st…
arw2019 Nov 24, 2020
719d3cd
feedback: revert changes to string_.py
arw2019 Nov 25, 2020
f1489c7
Merge branch 'master' of https://github.com/pandas-dev/pandas into st…
arw2019 Nov 25, 2020
0e84df8
remove fillna override from StringArray
arw2019 Nov 25, 2020
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
3 changes: 2 additions & 1 deletion pandas/core/arrays/string_.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,8 @@ def __setitem__(self, key, value):
super().__setitem__(key, value)

def fillna(self, value=None, method=None, limit=None):
# TODO: validate dtype
if not isinstance(value, str):
Copy link
Member

Choose a reason for hiding this comment

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

np.str_?

its a weird usage, but there's no reason why a user couldn't pass a n NA fill value.

Copy link
Member Author

Choose a reason for hiding this comment

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

added both, including the missing value

Copy link
Member

Choose a reason for hiding this comment

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

np.str_ is already be covered by str (it subclasses str):

In [13]: arr = np.array(["a"])

In [14]: type(arr[0])
Out[14]: numpy.str_

In [15]: isinstance(arr[0], str)
Out[15]: True

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah right. Got rid of the np.str_ check

raise TypeError(f"{value} is not a valid fill value; must be a string")
return super().fillna(value, method, limit)
Copy link
Member

Choose a reason for hiding this comment

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

does this method need to be overriden here at all?

Copy link
Member Author

Choose a reason for hiding this comment

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

it doesn't

Copy link
Member Author

Choose a reason for hiding this comment

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

pushed that now


def astype(self, dtype, copy=True):
Expand Down
10 changes: 10 additions & 0 deletions pandas/tests/arrays/string_/test_string.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,16 @@ def test_reduce_missing(skipna, dtype):
assert pd.isna(result)


def test_fillna_args():
arr = pd.array(["a", pd.NA], dtype="string")
arr.fillna(value="b")

arr = pd.array(["a", "b"], dtype="string")
msg = r"1 is not a valid fill value; must be a string"
with pytest.raises(TypeError, match=msg):
arr.fillna(value=1)


@td.skip_if_no("pyarrow", min_version="0.15.0")
def test_arrow_array(dtype):
# protocol added in 0.15.0
Expand Down