Skip to content

Fixed IntegerArray division by 0 #30183

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
merged 3 commits into from
Dec 11, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
1 change: 1 addition & 0 deletions doc/source/whatsnew/v1.0.0.rst
Original file line number Diff line number Diff line change
Expand Up @@ -856,6 +856,7 @@ Other
- Bug in :meth:`DataFrame.append` that raised ``IndexError`` when appending with empty list (:issue:`28769`)
- Fix :class:`AbstractHolidayCalendar` to return correct results for
years after 2030 (now goes up to 2200) (:issue:`27790`)
- Fixed :class:`IntegerArray` returning ``NA`` rather than ``inf`` for operations dividing by 0 (:issue:`27398`)
Copy link
Member

Choose a reason for hiding this comment

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

This is a bit confusingly worded I think, as I would interpret this that it now returns NaN and no longer inf, while it is the other way around?

Also NA -> NaN, as the result is about floats with NaNs

Copy link
Contributor

Choose a reason for hiding this comment

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

for non-trivially to understand changes we usually have a sub-section, but up to you.

- Bug in :meth:`Series.count` raises if use_inf_as_na is enabled (:issue:`29478`)


Expand Down
7 changes: 1 addition & 6 deletions pandas/core/arrays/integer.py
Original file line number Diff line number Diff line change
Expand Up @@ -700,11 +700,6 @@ def _maybe_mask_result(self, result, mask, other, op_name):
op_name : str
"""

# may need to fill infs
# and mask wraparound
if is_float_dtype(result):
mask |= (result == np.inf) | (result == -np.inf)
Copy link
Member

Choose a reason for hiding this comment

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

i think this line was at the root of #27829, so this might fix that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think you're correct.


# if we have a float operand we are by-definition
# a float result
# or our op is a divide
Expand Down Expand Up @@ -748,7 +743,7 @@ def integer_arithmetic_method(self, other):

# nans propagate
if mask is None:
mask = self._mask
mask = self._mask.copy()
else:
mask = self._mask | mask

Expand Down
11 changes: 11 additions & 0 deletions pandas/tests/arrays/test_integer.py
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,13 @@ def test_error(self, data, all_arithmetic_operators):
with pytest.raises(NotImplementedError):
opa(np.arange(len(s)).reshape(-1, len(s)))

def test_divide_by_zero(self):
# https://github.com/pandas-dev/pandas/issues/27398
a = pd.array([0, 1, -1, None], dtype="Int64")
result = a / 0
expected = np.array([np.nan, np.inf, -np.inf, np.nan])
tm.assert_numpy_array_equal(result, expected)

def test_pow(self):
# https://github.com/pandas-dev/pandas/issues/22022
a = integer_array([1, np.nan, np.nan, 1])
Expand Down Expand Up @@ -389,6 +396,10 @@ def test_compare_array(self, data, all_compare_operators):
other = pd.Series([0] * len(data))
self._compare_other(data, op_name, other)

def test_no_shared_mask(self, data):
result = data + 1
assert np.shares_memory(result._mask, data._mask) is False
Copy link
Member

Choose a reason for hiding this comment

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

i didnt know about shares_memory, neat!



class TestCasting:
@pytest.mark.parametrize("dropna", [True, False])
Expand Down