-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
implement assert_tzawareness_compat for DatetimeIndex #18376
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
Changes from 33 commits
97c32fd
2f19f51
f6a0e64
b18a006
623681f
28f8896
0f2287c
171238d
e95d75e
ba65aaf
49821d8
2be828e
b9e7c6d
eefadf7
470bd60
abe65f8
7cba103
92dfed4
2d68f01
712096b
f0c76ac
ddf1cea
3b4a01e
aeef3d7
cc5976a
662c447
123a52d
909d07a
f21f025
ead6e3a
a6008e0
0233a48
e2611dc
82ead56
5653c61
615aad5
31b7336
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,14 +13,14 @@ | |
_INT64_DTYPE, | ||
_NS_DTYPE, | ||
is_object_dtype, | ||
is_datetime64_dtype, | ||
is_datetime64_dtype, is_datetime64tz_dtype, | ||
is_datetimetz, | ||
is_dtype_equal, | ||
is_timedelta64_dtype, | ||
is_integer, | ||
is_float, | ||
is_integer_dtype, | ||
is_datetime64_ns_dtype, | ||
is_datetime64_ns_dtype, is_datetimelike, | ||
is_period_dtype, | ||
is_bool_dtype, | ||
is_string_like, | ||
|
@@ -106,8 +106,12 @@ def _dt_index_cmp(opname, cls, nat_result=False): | |
|
||
def wrapper(self, other): | ||
func = getattr(super(DatetimeIndex, self), opname) | ||
if (isinstance(other, datetime) or | ||
isinstance(other, compat.string_types)): | ||
|
||
if isinstance(other, (datetime, compat.string_types)): | ||
if isinstance(other, datetime): | ||
# GH#18435 strings get a pass from tzawareness compat | ||
self._assert_tzawareness_compat(other) | ||
|
||
other = _to_m8(other, tz=self.tz) | ||
result = func(other) | ||
if isna(other): | ||
|
@@ -117,6 +121,10 @@ def wrapper(self, other): | |
other = DatetimeIndex(other) | ||
elif not isinstance(other, (np.ndarray, Index, ABCSeries)): | ||
other = _ensure_datetime64(other) | ||
|
||
if is_datetimelike(other): | ||
self._assert_tzawareness_compat(other) | ||
|
||
result = func(np.asarray(other)) | ||
result = _values_from_object(result) | ||
|
||
|
@@ -652,6 +660,20 @@ def _simple_new(cls, values, name=None, freq=None, tz=None, | |
result._reset_identity() | ||
return result | ||
|
||
def _assert_tzawareness_compat(self, other): | ||
# adapted from _Timestamp._assert_tzawareness_compat | ||
other_tz = getattr(other, 'tzinfo', None) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is odd to use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That was my first thought too but There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. use .tz, you can simply wrap |
||
if is_datetime64tz_dtype(other): | ||
# Get tzinfo from Series dtype | ||
other_tz = other.dtype.tz | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think would make sense to make a single routine in tslib/timezones.pyx which does the tzawareness check (make it a function), then call it both from Timestamp and here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see the appeal in this but have some reservations. Do you mind saving this for the next round so I can give it some more thought? |
||
if self.tz is None: | ||
if other_tz is not None: | ||
raise TypeError('Cannot compare tz-naive and tz-aware ' | ||
'datetime-like objects.') | ||
elif other_tz is None: | ||
raise TypeError('Cannot compare tz-naive and tz-aware ' | ||
'datetime-like objects') | ||
|
||
@property | ||
def tzinfo(self): | ||
""" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,5 @@ | ||
import operator | ||
|
||
import pytest | ||
|
||
import numpy as np | ||
|
@@ -248,6 +250,42 @@ def test_append_join_nondatetimeindex(self): | |
# it works | ||
rng.join(idx, how='outer') | ||
|
||
@pytest.mark.parametrize('op', [operator.eq, operator.ne, | ||
operator.gt, operator.ge, | ||
operator.lt, operator.le]) | ||
def test_comparison_tzawareness_compat(self, op): | ||
# GH#18162 | ||
dr = pd.date_range('2016-01-01', periods=6) | ||
dz = dr.tz_localize('US/Pacific') | ||
|
||
with pytest.raises(TypeError): | ||
op(dr, dz) | ||
with pytest.raises(TypeError): | ||
op(dr, list(dz)) | ||
with pytest.raises(TypeError): | ||
op(dz, dr) | ||
with pytest.raises(TypeError): | ||
op(dz, list(dr)) | ||
|
||
# Check that there isn't a problem aware-aware and naive-naive do not | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. these tests should be in series (the latter ones). alternatively a better place might be tin test_base for these, where we already handle series & index tests. |
||
# raise | ||
assert (dr == dr).all() | ||
assert (dr == list(dr)).all() | ||
assert (dz == dz).all() | ||
assert (dz == list(dz)).all() | ||
|
||
# Check comparisons against scalar Timestamps | ||
ts = pd.Timestamp('2000-03-14 01:59') | ||
ts_tz = pd.Timestamp('2000-03-14 01:59', tz='Europe/Amsterdam') | ||
|
||
assert (dr > ts).all() | ||
with pytest.raises(TypeError): | ||
op(dr, ts_tz) | ||
|
||
assert (dz > ts_tz).all() | ||
with pytest.raises(TypeError): | ||
op(dz, ts) | ||
|
||
def test_comparisons_coverage(self): | ||
rng = date_range('1/1/2000', periods=10) | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move to conversaion