Skip to content

Commit

Permalink
BUG: Let IntervalIndex constructor override inferred closed (pandas-d…
Browse files Browse the repository at this point in the history
  • Loading branch information
jschendel authored and victor committed Sep 30, 2018
1 parent 2ddc724 commit 20b7b58
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 22 deletions.
7 changes: 7 additions & 0 deletions doc/source/whatsnew/v0.24.0.txt
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,13 @@ Strings
-
-

Interval
^^^^^^^^

- Bug in the :class:`IntervalIndex` constructor where the ``closed`` parameter did not always override the inferred ``closed`` (:issue:`19370`)
-
-

Indexing
^^^^^^^^

Expand Down
19 changes: 15 additions & 4 deletions pandas/_libs/interval.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -335,11 +335,17 @@ cdef class Interval(IntervalMixin):

@cython.wraparound(False)
@cython.boundscheck(False)
cpdef intervals_to_interval_bounds(ndarray intervals):
cpdef intervals_to_interval_bounds(ndarray intervals,
bint validate_closed=True):
"""
Parameters
----------
intervals: ndarray object array of Intervals / nulls
intervals : ndarray
object array of Intervals / nulls
validate_closed: boolean, default True
boolean indicating if all intervals must be closed on the same side.
Mismatching closed will raise if True, else return None for closed.
Returns
-------
Expand All @@ -353,6 +359,7 @@ cpdef intervals_to_interval_bounds(ndarray intervals):
object closed = None, interval
int64_t n = len(intervals)
ndarray left, right
bint seen_closed = False

left = np.empty(n, dtype=intervals.dtype)
right = np.empty(n, dtype=intervals.dtype)
Expand All @@ -370,10 +377,14 @@ cpdef intervals_to_interval_bounds(ndarray intervals):

left[i] = interval.left
right[i] = interval.right
if closed is None:
if not seen_closed:
seen_closed = True
closed = interval.closed
elif closed != interval.closed:
raise ValueError('intervals must all be closed on the same side')
closed = None
if validate_closed:
msg = 'intervals must all be closed on the same side'
raise ValueError(msg)

return left, right, closed

Expand Down
14 changes: 3 additions & 11 deletions pandas/core/indexes/interval.py
Original file line number Diff line number Diff line change
Expand Up @@ -233,24 +233,16 @@ def __new__(cls, data, closed=None, dtype=None, copy=False,
if isinstance(data, IntervalIndex):
left = data.left
right = data.right
closed = data.closed
closed = closed or data.closed
else:

# don't allow scalars
if is_scalar(data):
cls._scalar_data_error(data)

data = maybe_convert_platform_interval(data)
left, right, infer_closed = intervals_to_interval_bounds(data)

if (com._all_not_none(closed, infer_closed) and
closed != infer_closed):
# GH 18421
msg = ("conflicting values for closed: constructor got "
"'{closed}', inferred from data '{infer_closed}'"
.format(closed=closed, infer_closed=infer_closed))
raise ValueError(msg)

left, right, infer_closed = intervals_to_interval_bounds(
data, validate_closed=closed is None)
closed = closed or infer_closed

return cls._simple_new(left, right, closed, name, copy=copy,
Expand Down
26 changes: 19 additions & 7 deletions pandas/tests/indexes/interval/test_construction.py
Original file line number Diff line number Diff line change
Expand Up @@ -312,13 +312,7 @@ def test_generic_errors(self, constructor):
pass

def test_constructor_errors(self, constructor):
# mismatched closed inferred from intervals vs constructor.
ivs = [Interval(0, 1, closed='both'), Interval(1, 2, closed='both')]
msg = 'conflicting values for closed'
with tm.assert_raises_regex(ValueError, msg):
constructor(ivs, closed='neither')

# mismatched closed within intervals
# mismatched closed within intervals with no constructor override
ivs = [Interval(0, 1, closed='right'), Interval(2, 3, closed='left')]
msg = 'intervals must all be closed on the same side'
with tm.assert_raises_regex(ValueError, msg):
Expand All @@ -336,6 +330,24 @@ def test_constructor_errors(self, constructor):
with tm.assert_raises_regex(TypeError, msg):
constructor([0, 1])

@pytest.mark.parametrize('data, closed', [
([], 'both'),
([np.nan, np.nan], 'neither'),
([Interval(0, 3, closed='neither'),
Interval(2, 5, closed='neither')], 'left'),
([Interval(0, 3, closed='left'),
Interval(2, 5, closed='right')], 'neither'),
(IntervalIndex.from_breaks(range(5), closed='both'), 'right')])
def test_override_inferred_closed(self, constructor, data, closed):
# GH 19370
if isinstance(data, IntervalIndex):
tuples = data.to_tuples()
else:
tuples = [(iv.left, iv.right) if notna(iv) else iv for iv in data]
expected = IntervalIndex.from_tuples(tuples, closed=closed)
result = constructor(data, closed=closed)
tm.assert_index_equal(result, expected)


class TestFromIntervals(TestClassConstructors):
"""
Expand Down

0 comments on commit 20b7b58

Please sign in to comment.