Skip to content

Commit

Permalink
Revert inclusive default change of IntervalDtype (#47367)
Browse files Browse the repository at this point in the history
  • Loading branch information
phofl authored Jul 6, 2022
1 parent 37e6239 commit d9dd128
Show file tree
Hide file tree
Showing 13 changed files with 217 additions and 96 deletions.
1 change: 1 addition & 0 deletions doc/source/reference/arrays.rst
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,7 @@ Properties
:toctree: api/

Interval.inclusive
Interval.closed
Interval.closed_left
Interval.closed_right
Interval.is_empty
Expand Down
2 changes: 2 additions & 0 deletions pandas/_libs/interval.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@ class Interval(IntervalMixin, Generic[_OrderableT]):
def right(self: Interval[_OrderableT]) -> _OrderableT: ...
@property
def inclusive(self) -> IntervalClosedType: ...
@property
def closed(self) -> IntervalClosedType: ...
mid: _MidDescriptor
length: _LengthDescriptor
def __init__(
Expand Down
21 changes: 19 additions & 2 deletions pandas/_libs/interval.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ from cpython.datetime cimport (
import_datetime,
)

from pandas.util._exceptions import find_stack_level

import_datetime()

cimport cython
Expand Down Expand Up @@ -229,7 +231,7 @@ def _warning_interval(inclusive: str | None = None, closed: None | lib.NoDefault
stacklevel=2,
)
if closed is None:
inclusive = "both"
inclusive = "right"
elif closed in ("both", "neither", "left", "right"):
inclusive = closed
else:
Expand Down Expand Up @@ -364,7 +366,7 @@ cdef class Interval(IntervalMixin):
inclusive, closed = _warning_interval(inclusive, closed)

if inclusive is None:
inclusive = "both"
inclusive = "right"

if inclusive not in VALID_CLOSED:
raise ValueError(f"invalid option for 'inclusive': {inclusive}")
Expand All @@ -379,6 +381,21 @@ cdef class Interval(IntervalMixin):
self.right = right
self.inclusive = inclusive

@property
def closed(self):
"""
Whether the interval is closed on the left-side, right-side, both or
neither.
.. deprecated:: 1.5.0
"""
warnings.warn(
"Attribute `closed` is deprecated in favor of `inclusive`.",
FutureWarning,
stacklevel=find_stack_level(),
)
return self.inclusive

def _validate_endpoint(self, endpoint):
# GH 23013
if not (is_integer_object(endpoint) or is_float_object(endpoint) or
Expand Down
2 changes: 1 addition & 1 deletion pandas/_libs/intervaltree.pxi.in
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ cdef class IntervalTree(IntervalMixin):
inclusive, closed = _warning_interval(inclusive, closed)

if inclusive is None:
inclusive = "both"
inclusive = "right"

if inclusive not in ['left', 'right', 'both', 'neither']:
raise ValueError("invalid option for 'inclusive': %s" % inclusive)
Expand Down
21 changes: 12 additions & 9 deletions pandas/core/arrays/arrow/_arrow_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,8 @@
import numpy as np
import pyarrow

from pandas._libs import lib
from pandas._libs.interval import _warning_interval
from pandas.errors import PerformanceWarning
from pandas.util._decorators import deprecate_kwarg
from pandas.util._exceptions import find_stack_level

from pandas.core.arrays.interval import VALID_CLOSED
Expand Down Expand Up @@ -105,15 +104,10 @@ def to_pandas_dtype(self):


class ArrowIntervalType(pyarrow.ExtensionType):
def __init__(
self,
subtype,
inclusive: str | None = None,
closed: None | lib.NoDefault = lib.no_default,
) -> None:
@deprecate_kwarg(old_arg_name="closed", new_arg_name="inclusive")
def __init__(self, subtype, inclusive: str) -> None:
# attributes need to be set first before calling
# super init (as that calls serialize)
inclusive, closed = _warning_interval(inclusive, closed)
assert inclusive in VALID_CLOSED
self._closed = inclusive
if not isinstance(subtype, pyarrow.DataType):
Expand All @@ -131,6 +125,15 @@ def subtype(self):
def inclusive(self):
return self._closed

@property
def closed(self):
warnings.warn(
"Attribute `closed` is deprecated in favor of `inclusive`.",
FutureWarning,
stacklevel=find_stack_level(),
)
return self._closed

def __arrow_ext_serialize__(self):
metadata = {"subtype": str(self.subtype), "inclusive": self.inclusive}
return json.dumps(metadata).encode()
Expand Down
46 changes: 36 additions & 10 deletions pandas/core/arrays/interval.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
cast,
overload,
)
import warnings

import numpy as np

Expand All @@ -25,7 +26,6 @@
VALID_CLOSED,
Interval,
IntervalMixin,
_warning_interval,
intervals_to_interval_bounds,
)
from pandas._libs.missing import NA
Expand All @@ -43,8 +43,10 @@
from pandas.errors import IntCastingNaNError
from pandas.util._decorators import (
Appender,
deprecate_kwarg,
deprecate_nonkeyword_arguments,
)
from pandas.util._exceptions import find_stack_level

from pandas.core.dtypes.cast import LossySetitemError
from pandas.core.dtypes.common import (
Expand Down Expand Up @@ -220,16 +222,15 @@ def ndim(self) -> Literal[1]:
# ---------------------------------------------------------------------
# Constructors

@deprecate_kwarg(old_arg_name="closed", new_arg_name="inclusive")
def __new__(
cls: type[IntervalArrayT],
data,
inclusive: str | None = None,
closed: None | lib.NoDefault = lib.no_default,
dtype: Dtype | None = None,
copy: bool = False,
verify_integrity: bool = True,
):
inclusive, closed = _warning_interval(inclusive, closed)

data = extract_array(data, extract_numpy=True)

Expand Down Expand Up @@ -267,24 +268,22 @@ def __new__(
)

@classmethod
@deprecate_kwarg(old_arg_name="closed", new_arg_name="inclusive")
def _simple_new(
cls: type[IntervalArrayT],
left,
right,
inclusive=None,
closed: None | lib.NoDefault = lib.no_default,
copy: bool = False,
dtype: Dtype | None = None,
verify_integrity: bool = True,
) -> IntervalArrayT:
result = IntervalMixin.__new__(cls)

inclusive, closed = _warning_interval(inclusive, closed)

if inclusive is None and isinstance(dtype, IntervalDtype):
inclusive = dtype.inclusive

inclusive = inclusive or "both"
inclusive = inclusive or "right"

left = ensure_index(left, copy=copy)
right = ensure_index(right, copy=copy)
Expand Down Expand Up @@ -424,13 +423,17 @@ def _from_factorized(
),
}
)
@deprecate_kwarg(old_arg_name="closed", new_arg_name="inclusive")
def from_breaks(
cls: type[IntervalArrayT],
breaks,
inclusive="both",
inclusive: IntervalClosedType | None = None,
copy: bool = False,
dtype: Dtype | None = None,
) -> IntervalArrayT:
if inclusive is None:
inclusive = "right"

breaks = _maybe_convert_platform_interval(breaks)

return cls.from_arrays(
Expand Down Expand Up @@ -501,14 +504,19 @@ def from_breaks(
),
}
)
@deprecate_kwarg(old_arg_name="closed", new_arg_name="inclusive")
def from_arrays(
cls: type[IntervalArrayT],
left,
right,
inclusive="both",
inclusive: IntervalClosedType | None = None,
copy: bool = False,
dtype: Dtype | None = None,
) -> IntervalArrayT:

if inclusive is None:
inclusive = "right"

left = _maybe_convert_platform_interval(left)
right = _maybe_convert_platform_interval(right)

Expand Down Expand Up @@ -570,13 +578,17 @@ def from_arrays(
),
}
)
@deprecate_kwarg(old_arg_name="closed", new_arg_name="inclusive")
def from_tuples(
cls: type[IntervalArrayT],
data,
inclusive="both",
inclusive=None,
copy: bool = False,
dtype: Dtype | None = None,
) -> IntervalArrayT:
if inclusive is None:
inclusive = "right"

if len(data):
left, right = [], []
else:
Expand Down Expand Up @@ -1355,6 +1367,19 @@ def inclusive(self) -> IntervalClosedType:
"""
return self.dtype.inclusive

@property
def closed(self) -> IntervalClosedType:
"""
Whether the intervals are closed on the left-side, right-side, both or
neither.
"""
warnings.warn(
"Attribute `closed` is deprecated in favor of `inclusive`.",
FutureWarning,
stacklevel=find_stack_level(),
)
return self.dtype.inclusive

_interval_shared_docs["set_closed"] = textwrap.dedent(
"""
Return an %(klass)s identical to the current one, but closed on the
Expand Down Expand Up @@ -1395,6 +1420,7 @@ def inclusive(self) -> IntervalClosedType:
),
}
)
@deprecate_kwarg(old_arg_name="closed", new_arg_name="inclusive")
def set_closed(
self: IntervalArrayT, inclusive: IntervalClosedType
) -> IntervalArrayT:
Expand Down
11 changes: 11 additions & 0 deletions pandas/core/dtypes/dtypes.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
MutableMapping,
cast,
)
import warnings

import numpy as np
import pytz
Expand Down Expand Up @@ -40,6 +41,7 @@
npt,
type_t,
)
from pandas.util._exceptions import find_stack_level

from pandas.core.dtypes.base import (
ExtensionDtype,
Expand Down Expand Up @@ -1176,6 +1178,15 @@ def _can_hold_na(self) -> bool:
def inclusive(self):
return self._closed

@property
def closed(self):
warnings.warn(
"Attribute `closed` is deprecated in favor of `inclusive`.",
FutureWarning,
stacklevel=find_stack_level(),
)
return self._closed

@property
def subtype(self):
"""
Expand Down
Loading

0 comments on commit d9dd128

Please sign in to comment.