Skip to content

BUG: Interval scalar arithmetic operations reset closed #22331

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 1 commit into from
Aug 14, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
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
2 changes: 1 addition & 1 deletion doc/source/whatsnew/v0.24.0.txt
Original file line number Diff line number Diff line change
Expand Up @@ -600,7 +600,7 @@ Interval

- Bug in the :class:`IntervalIndex` constructor where the ``closed`` parameter did not always override the inferred ``closed`` (:issue:`19370`)
- Bug in the ``IntervalIndex`` repr where a trailing comma was missing after the list of intervals (:issue:`20611`)
-
- Bug in :class:`Interval` where scalar arithmetic operations did not retain the ``closed`` value (:issue:`22313`)
-

Indexing
Expand Down
17 changes: 9 additions & 8 deletions pandas/_libs/interval.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -326,36 +326,37 @@ cdef class Interval(IntervalMixin):

def __add__(self, y):
if isinstance(y, numbers.Number):
return Interval(self.left + y, self.right + y)
return Interval(self.left + y, self.right + y, closed=self.closed)
elif isinstance(y, Interval) and isinstance(self, numbers.Number):
return Interval(y.left + self, y.right + self)
return Interval(y.left + self, y.right + self, closed=y.closed)
return NotImplemented

def __sub__(self, y):
if isinstance(y, numbers.Number):
return Interval(self.left - y, self.right - y)
return Interval(self.left - y, self.right - y, closed=self.closed)
return NotImplemented

def __mul__(self, y):
if isinstance(y, numbers.Number):
return Interval(self.left * y, self.right * y)
return Interval(self.left * y, self.right * y, closed=self.closed)
elif isinstance(y, Interval) and isinstance(self, numbers.Number):
return Interval(y.left * self, y.right * self)
return Interval(y.left * self, y.right * self, closed=y.closed)
return NotImplemented

def __div__(self, y):
if isinstance(y, numbers.Number):
return Interval(self.left / y, self.right / y)
return Interval(self.left / y, self.right / y, closed=self.closed)
return NotImplemented

def __truediv__(self, y):
if isinstance(y, numbers.Number):
return Interval(self.left / y, self.right / y)
return Interval(self.left / y, self.right / y, closed=self.closed)
return NotImplemented

def __floordiv__(self, y):
if isinstance(y, numbers.Number):
return Interval(self.left // y, self.right // y)
return Interval(
self.left // y, self.right // y, closed=self.closed)
return NotImplemented


Expand Down
100 changes: 62 additions & 38 deletions pandas/tests/scalar/interval/test_interval.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,79 +109,103 @@ def test_length_errors(self, left, right):
with tm.assert_raises_regex(TypeError, msg):
iv.length

def test_math_add(self, interval):
Copy link
Member Author

@jschendel jschendel Aug 14, 2018

Choose a reason for hiding this comment

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

I could have parametrized the interval fixture over closed but it would have blown up a lot of other tests that use this fixture. Wanted this PR to be easy to review, so I only made changes related to the arithmetic operations.

Will clean up these tests as a whole in a follow up. Found a few other bugs when reviewing this code and will want to document/fix those and take them into account prior to cleaning.

expected = Interval(1, 2)
actual = interval + 1
assert expected == actual
def test_math_add(self, closed):
interval = Interval(0, 1, closed=closed)
expected = Interval(1, 2, closed=closed)

expected = Interval(1, 2)
actual = 1 + interval
assert expected == actual
result = interval + 1
assert result == expected

result = 1 + interval
assert result == expected

actual = interval
actual += 1
assert expected == actual
result = interval
result += 1
assert result == expected

msg = r"unsupported operand type\(s\) for \+"
with tm.assert_raises_regex(TypeError, msg):
interval + Interval(1, 2)
interval + interval

with tm.assert_raises_regex(TypeError, msg):
interval + 'foo'

def test_math_sub(self, interval):
expected = Interval(-1, 0)
actual = interval - 1
assert expected == actual
def test_math_sub(self, closed):
interval = Interval(0, 1, closed=closed)
expected = Interval(-1, 0, closed=closed)

result = interval - 1
assert result == expected

actual = interval
actual -= 1
assert expected == actual
result = interval
result -= 1
assert result == expected

msg = r"unsupported operand type\(s\) for -"
with tm.assert_raises_regex(TypeError, msg):
interval - Interval(1, 2)
interval - interval

with tm.assert_raises_regex(TypeError, msg):
interval - 'foo'

def test_math_mult(self, interval):
expected = Interval(0, 2)
actual = interval * 2
assert expected == actual
def test_math_mult(self, closed):
interval = Interval(0, 1, closed=closed)
expected = Interval(0, 2, closed=closed)

result = interval * 2
assert result == expected

expected = Interval(0, 2)
actual = 2 * interval
assert expected == actual
result = 2 * interval
assert result == expected

actual = interval
actual *= 2
assert expected == actual
result = interval
result *= 2
assert result == expected

msg = r"unsupported operand type\(s\) for \*"
with tm.assert_raises_regex(TypeError, msg):
interval * Interval(1, 2)
interval * interval

msg = r"can\'t multiply sequence by non-int"
with tm.assert_raises_regex(TypeError, msg):
interval * 'foo'

def test_math_div(self, interval):
expected = Interval(0, 0.5)
actual = interval / 2.0
assert expected == actual
def test_math_div(self, closed):
interval = Interval(0, 1, closed=closed)
expected = Interval(0, 0.5, closed=closed)

actual = interval
actual /= 2.0
assert expected == actual
result = interval / 2.0
assert result == expected

result = interval
result /= 2.0
assert result == expected

msg = r"unsupported operand type\(s\) for /"
with tm.assert_raises_regex(TypeError, msg):
interval / Interval(1, 2)
interval / interval

with tm.assert_raises_regex(TypeError, msg):
interval / 'foo'

def test_math_floordiv(self, closed):
interval = Interval(1, 2, closed=closed)
expected = Interval(0, 1, closed=closed)

result = interval // 2
assert result == expected

result = interval
result //= 2
assert result == expected

msg = r"unsupported operand type\(s\) for //"
with tm.assert_raises_regex(TypeError, msg):
interval // interval

with tm.assert_raises_regex(TypeError, msg):
interval // 'foo'

def test_constructor_errors(self):
msg = "invalid option for 'closed': foo"
with tm.assert_raises_regex(ValueError, msg):
Expand Down