Skip to content

REF: use shareable code for DTI/TDI.insert #30806

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 6 commits into from
Jan 9, 2020
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
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 @@ -872,6 +872,7 @@ Datetimelike
- Bug in :func:`pandas.to_datetime` when called with ``Series`` storing ``IntegerArray`` raising ``TypeError`` instead of returning ``Series`` (:issue:`30050`)
- Bug in :func:`date_range` with custom business hours as ``freq`` and given number of ``periods`` (:issue:`30593`)
- Bug in :class:`PeriodIndex` comparisons with incorrectly casting integers to :class:`Period` objects, inconsistent with the :class:`Period` comparison behavior (:issue:`30722`)
- Bug in :meth:`DatetimeIndex.insert` raising a ``ValueError`` instead of a ``TypeError`` when trying to insert a timezone-aware :class:`Timestamp` into a timezone-naive :class:`DatetimeIndex`, or vice-versa (:issue:`30806`)

Timedelta
^^^^^^^^^
Expand Down
21 changes: 11 additions & 10 deletions pandas/core/indexes/datetimes.py
Original file line number Diff line number Diff line change
Expand Up @@ -887,7 +887,9 @@ def insert(self, loc, item):
-------
new_index : Index
"""
if is_valid_nat_for_dtype(item, self.dtype):
if isinstance(item, self._data._recognized_scalars):
item = self._data._scalar_type(item)
elif is_valid_nat_for_dtype(item, self.dtype):
# GH 18295
item = self._na_value
elif is_scalar(item) and isna(item):
Expand All @@ -897,11 +899,8 @@ def insert(self, loc, item):
)

freq = None

if isinstance(item, (datetime, np.datetime64)):
self._assert_can_do_op(item)
if not self._has_same_tz(item) and not isna(item):
raise ValueError("Passed item and index have different timezone")
if isinstance(item, self._data._scalar_type) or item is NaT:
self._data._check_compatible_with(item, setitem=True)

# check freq can be preserved on edge cases
if self.size and self.freq is not None:
Expand All @@ -911,19 +910,21 @@ def insert(self, loc, item):
freq = self.freq
elif (loc == len(self)) and item - self.freq == self[-1]:
freq = self.freq
item = _to_M8(item, tz=self.tz)
item = item.asm8

try:
new_dates = np.concatenate(
new_i8s = np.concatenate(
(self[:loc].asi8, [item.view(np.int64)], self[loc:].asi8)
)
return self._shallow_copy(new_dates, freq=freq)
return self._shallow_copy(new_i8s, freq=freq)
except (AttributeError, TypeError):

# fall back to object index
if isinstance(item, str):
return self.astype(object).insert(loc, item)
raise TypeError("cannot insert DatetimeIndex with incompatible label")
raise TypeError(
f"cannot insert {type(self).__name__} with incompatible label"
)

def indexer_at_time(self, time, asof=False):
"""
Expand Down
22 changes: 13 additions & 9 deletions pandas/core/indexes/timedeltas.py
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,7 @@ def insert(self, loc, item):
"""
# try to convert if possible
if isinstance(item, self._data._recognized_scalars):
item = Timedelta(item)
item = self._data._scalar_type(item)
elif is_valid_nat_for_dtype(item, self.dtype):
# GH 18295
item = self._na_value
Expand All @@ -398,28 +398,32 @@ def insert(self, loc, item):
)

freq = None
if isinstance(item, Timedelta) or (is_scalar(item) and isna(item)):
if isinstance(item, self._data._scalar_type) or item is NaT:
self._data._check_compatible_with(item, setitem=True)

# check freq can be preserved on edge cases
if self.freq is not None:
if (loc == 0 or loc == -len(self)) and item + self.freq == self[0]:
if self.size and self.freq is not None:
if item is NaT:
pass
elif (loc == 0 or loc == -len(self)) and item + self.freq == self[0]:
freq = self.freq
elif (loc == len(self)) and item - self.freq == self[-1]:
freq = self.freq
item = Timedelta(item).asm8.view(_TD_DTYPE)
item = item.asm8

try:
new_tds = np.concatenate(
new_i8s = np.concatenate(
(self[:loc].asi8, [item.view(np.int64)], self[loc:].asi8)
)
return self._shallow_copy(new_tds, freq=freq)

return self._shallow_copy(new_i8s, freq=freq)
except (AttributeError, TypeError):

# fall back to object index
if isinstance(item, str):
return self.astype(object).insert(loc, item)
raise TypeError("cannot insert TimedeltaIndex with incompatible label")
raise TypeError(
f"cannot insert {type(self).__name__} with incompatible label"
)


TimedeltaIndex._add_comparison_ops()
Expand Down
3 changes: 1 addition & 2 deletions pandas/tests/arithmetic/test_datetime64.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
date_range,
)
import pandas._testing as tm
from pandas.core.indexes.datetimes import _to_M8
from pandas.core.ops import roperator
from pandas.tests.arithmetic.common import (
assert_invalid_addsub_type,
Expand Down Expand Up @@ -341,7 +340,7 @@ class TestDatetimeIndexComparisons:
def test_comparators(self, op):
index = tm.makeDateIndex(100)
element = index[len(index) // 2]
element = _to_M8(element)
element = Timestamp(element).to_datetime64()

arr = np.array(index)
arr_result = op(arr, element)
Expand Down
4 changes: 2 additions & 2 deletions pandas/tests/indexes/datetimes/test_indexing.py
Original file line number Diff line number Diff line change
Expand Up @@ -434,9 +434,9 @@ def test_insert(self):

# see gh-7299
idx = date_range("1/1/2000", periods=3, freq="D", tz="Asia/Tokyo", name="idx")
with pytest.raises(ValueError):
with pytest.raises(TypeError, match="Cannot compare tz-naive and tz-aware"):
idx.insert(3, pd.Timestamp("2000-01-04"))
with pytest.raises(ValueError):
with pytest.raises(TypeError, match="Cannot compare tz-naive and tz-aware"):
idx.insert(3, datetime(2000, 1, 4))
with pytest.raises(ValueError):
idx.insert(3, pd.Timestamp("2000-01-04", tz="US/Eastern"))
Expand Down
14 changes: 10 additions & 4 deletions pandas/tests/indexing/test_coercion.py
Original file line number Diff line number Diff line change
Expand Up @@ -432,13 +432,19 @@ def test_insert_index_datetimes(self, fill_val, exp_dtype):
)
self._assert_insert_conversion(obj, fill_val, exp, exp_dtype)

msg = "Passed item and index have different timezone"
if fill_val.tz:
with pytest.raises(ValueError, match=msg):
msg = "Cannot compare tz-naive and tz-aware"
Copy link
Contributor

Choose a reason for hiding this comment

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

@TomAugspurger is this what you are talking about? this I think was just wrong before, not an API change.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah. I see now that we raise TypeError for Timestamp == Timestamp, which I think is incorrect but not worth changing.

I still thing think this merits a release note.

Copy link
Contributor

Choose a reason for hiding this comment

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

sure, @jbrockmendel can you add for what is the user viisble change.

Copy link
Member Author

Choose a reason for hiding this comment

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

updated with whatsnew + green

with pytest.raises(TypeError, match=msg):
obj.insert(1, pd.Timestamp("2012-01-01"))

with pytest.raises(ValueError, match=msg):
obj.insert(1, pd.Timestamp("2012-01-01", tz="Asia/Tokyo"))
msg = "Timezones don't match"
with pytest.raises(ValueError, match=msg):
obj.insert(1, pd.Timestamp("2012-01-01", tz="Asia/Tokyo"))

else:
msg = "Cannot compare tz-naive and tz-aware"
with pytest.raises(TypeError, match=msg):
obj.insert(1, pd.Timestamp("2012-01-01", tz="Asia/Tokyo"))

msg = "cannot insert DatetimeIndex with incompatible label"
with pytest.raises(TypeError, match=msg):
Expand Down