Skip to content

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

Merged
merged 37 commits into from
Jan 6, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
97c32fd
implement tzawareness_compat for datetimeIndex
jbrockmendel Nov 19, 2017
2f19f51
Merge branch 'master' of https://github.com/pandas-dev/pandas into tz…
jbrockmendel Nov 19, 2017
f6a0e64
convert before asserting tzawareness_compat
jbrockmendel Nov 20, 2017
b18a006
Merge branch 'master' of https://github.com/pandas-dev/pandas into tz…
jbrockmendel Nov 23, 2017
623681f
Merge branch 'master' of https://github.com/pandas-dev/pandas into tz…
jbrockmendel Nov 24, 2017
28f8896
Update test to disallow tzaware vs tznaive comparison
jbrockmendel Nov 24, 2017
0f2287c
flake8 fixup
jbrockmendel Nov 24, 2017
171238d
Fix typo
jbrockmendel Nov 25, 2017
e95d75e
Move part of test per reviewer request
jbrockmendel Nov 25, 2017
ba65aaf
update test to include scalar comparisons
jbrockmendel Nov 25, 2017
49821d8
Merge branch 'master' of https://github.com/pandas-dev/pandas into tz…
jbrockmendel Nov 25, 2017
2be828e
Merge branch 'master' of https://github.com/pandas-dev/pandas into tz…
jbrockmendel Nov 25, 2017
b9e7c6d
revert hack, xfail test
jbrockmendel Nov 25, 2017
eefadf7
parametrize tests, comment for xfail
jbrockmendel Nov 26, 2017
470bd60
Merge branch 'master' of https://github.com/pandas-dev/pandas into tz…
jbrockmendel Nov 26, 2017
abe65f8
remove debugging prints
jbrockmendel Dec 6, 2017
7cba103
rearrange isinstance checks per suggestion
jbrockmendel Dec 6, 2017
92dfed4
Merge branch 'master' of https://github.com/pandas-dev/pandas into tz…
jbrockmendel Dec 6, 2017
2d68f01
add whatsnew note
jbrockmendel Dec 6, 2017
712096b
Merge branch 'master' of https://github.com/pandas-dev/pandas into tz…
jbrockmendel Dec 7, 2017
f0c76ac
Merge branch 'master' of https://github.com/pandas-dev/pandas into tz…
jbrockmendel Dec 7, 2017
ddf1cea
fix duplicate line from merge mixup
jbrockmendel Dec 7, 2017
3b4a01e
put comment into xfail message
jbrockmendel Dec 7, 2017
aeef3d7
Fixup missing keyword
jbrockmendel Dec 7, 2017
cc5976a
Merge branch 'master' into PR_TOOL_MERGE_PR_18376
jreback Dec 12, 2017
662c447
Merge branch 'master' of https://github.com/pandas-dev/pandas into tz…
jbrockmendel Dec 12, 2017
123a52d
Merge branch 'tzbugs' of https://github.com/jbrockmendel/pandas into …
jbrockmendel Dec 12, 2017
909d07a
Merge branch 'master' of https://github.com/pandas-dev/pandas into tz…
jbrockmendel Dec 14, 2017
f21f025
give strings a free pass from tzawareness compat
jbrockmendel Dec 16, 2017
ead6e3a
Merge branch 'master' of https://github.com/pandas-dev/pandas into tz…
jbrockmendel Dec 21, 2017
a6008e0
add list test cases
jbrockmendel Jan 4, 2018
0233a48
Merge branch 'master' of https://github.com/pandas-dev/pandas into tz…
jbrockmendel Jan 4, 2018
e2611dc
follow precedent to separate out xfailed tests
jbrockmendel Jan 4, 2018
82ead56
Merge branch 'master' of https://github.com/pandas-dev/pandas into tz…
jbrockmendel Jan 5, 2018
5653c61
move whatsnew to conversion
jbrockmendel Jan 5, 2018
615aad5
Merge branch 'master' of https://github.com/pandas-dev/pandas into tz…
jbrockmendel Jan 5, 2018
31b7336
Merge branch 'master' of https://github.com/pandas-dev/pandas into tz…
jbrockmendel Jan 5, 2018
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/v0.23.0.txt
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,7 @@ Conversion
- Bug in :class:`Series`` with ``dtype='timedelta64[ns]`` where addition or subtraction of ``TimedeltaIndex`` had results cast to ``dtype='int64'`` (:issue:`17250`)
- Bug in :class:`TimedeltaIndex` where division by a ``Series`` would return a ``TimedeltaIndex`` instead of a ``Series`` (issue:`19042`)
- Bug in :class:`Series` with ``dtype='timedelta64[ns]`` where addition or subtraction of ``TimedeltaIndex`` could return a ``Series`` with an incorrect name (issue:`19043`)
- Fixed bug where comparing :class:`DatetimeIndex` failed to raise ``TypeError`` when attempting to compare timezone-aware and timezone-naive datetimelike objects (:issue:`18162`)
-

Indexing
Expand Down
30 changes: 26 additions & 4 deletions pandas/core/indexes/datetimes.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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):
Expand All @@ -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)

Expand Down Expand Up @@ -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)
Copy link
Contributor

Choose a reason for hiding this comment

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

this is odd to use tzinfo, these are already wrapped scalars, or an index type, so .tz is appropriate

Copy link
Member Author

Choose a reason for hiding this comment

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

That was my first thought too but other could be a raw datetime.

Copy link
Contributor

Choose a reason for hiding this comment

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

use .tz, you can simply wrap other = pd.Timestamp(other)

if is_datetime64tz_dtype(other):
# Get tzinfo from Series dtype
other_tz = other.dtype.tz
Copy link
Contributor

Choose a reason for hiding this comment

The 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.

Copy link
Member Author

Choose a reason for hiding this comment

The 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):
"""
Expand Down
38 changes: 38 additions & 0 deletions pandas/tests/indexes/datetimes/test_datetime.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import operator

import pytest

import numpy as np
Expand Down Expand Up @@ -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
# 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)

Expand Down
20 changes: 20 additions & 0 deletions pandas/tests/indexes/test_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -2262,6 +2262,26 @@ def test_intersect_str_dates(self):

assert len(res) == 0

@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')

# Check that there isn't a problem aware-aware and naive-naive do not
# raise
naive_series = Series(dr)
aware_series = Series(dz)
with pytest.raises(TypeError):
op(dz, naive_series)
with pytest.raises(TypeError):
op(dr, aware_series)

# TODO: implement _assert_tzawareness_compat for the reverse
# comparison with the Series on the left-hand side


class TestIndexUtils(object):

Expand Down
34 changes: 34 additions & 0 deletions pandas/tests/indexing/test_coercion.py
Original file line number Diff line number Diff line change
Expand Up @@ -821,6 +821,9 @@ def test_replace_series(self, how, to_key, from_key):
if (from_key.startswith('datetime') and to_key.startswith('datetime')):
# tested below
return
elif from_key in ['datetime64[ns, US/Eastern]', 'datetime64[ns, UTC]']:
# tested below
return

if how == 'dict':
replacer = dict(zip(self.rep[from_key], self.rep[to_key]))
Expand Down Expand Up @@ -849,6 +852,37 @@ def test_replace_series(self, how, to_key, from_key):

tm.assert_series_equal(result, exp)

# TODO(jbrockmendel) commented out to only have a single xfail printed
@pytest.mark.xfail(reason='GH #18376, tzawareness-compat bug '
'in BlockManager.replace_list')
# @pytest.mark.parametrize('how', ['dict', 'series'])
# @pytest.mark.parametrize('to_key', ['timedelta64[ns]', 'bool', 'object',
# 'complex128', 'float64', 'int64'])
# @pytest.mark.parametrize('from_key', ['datetime64[ns, UTC]',
# 'datetime64[ns, US/Eastern]'])
# def test_replace_series_datetime_tz(self, how, to_key, from_key):
def test_replace_series_datetime_tz(self):
how = 'series'
from_key = 'datetime64[ns, US/Eastern]'
to_key = 'timedelta64[ns]'

index = pd.Index([3, 4], name='xxx')
obj = pd.Series(self.rep[from_key], index=index, name='yyy')
assert obj.dtype == from_key

if how == 'dict':
replacer = dict(zip(self.rep[from_key], self.rep[to_key]))
elif how == 'series':
replacer = pd.Series(self.rep[to_key], index=self.rep[from_key])
else:
raise ValueError

result = obj.replace(replacer)
exp = pd.Series(self.rep[to_key], index=index, name='yyy')
assert exp.dtype == to_key

tm.assert_series_equal(result, exp)

# TODO(jreback) commented out to only have a single xfail printed
@pytest.mark.xfail(reason="different tz, "
"currently mask_missing raises SystemError")
Expand Down
14 changes: 14 additions & 0 deletions pandas/tests/series/test_indexing.py
Original file line number Diff line number Diff line change
Expand Up @@ -450,6 +450,13 @@ def test_getitem_setitem_datetimeindex(self):

lb = "1990-01-01 04:00:00"
rb = "1990-01-01 07:00:00"
# GH#18435 strings get a pass from tzawareness compat
result = ts[(ts.index >= lb) & (ts.index <= rb)]
expected = ts[4:8]
assert_series_equal(result, expected)

lb = "1990-01-01 04:00:00-0500"
rb = "1990-01-01 07:00:00-0500"
result = ts[(ts.index >= lb) & (ts.index <= rb)]
expected = ts[4:8]
assert_series_equal(result, expected)
Expand All @@ -475,6 +482,13 @@ def test_getitem_setitem_datetimeindex(self):

lb = datetime(1990, 1, 1, 4)
rb = datetime(1990, 1, 1, 7)
with pytest.raises(TypeError):
# tznaive vs tzaware comparison is invalid
# see GH#18376, GH#18162
ts[(ts.index >= lb) & (ts.index <= rb)]

lb = pd.Timestamp(datetime(1990, 1, 1, 4)).tz_localize(rng.tzinfo)
rb = pd.Timestamp(datetime(1990, 1, 1, 7)).tz_localize(rng.tzinfo)
result = ts[(ts.index >= lb) & (ts.index <= rb)]
expected = ts[4:8]
assert_series_equal(result, expected)
Expand Down
2 changes: 1 addition & 1 deletion pandas/tests/test_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ def __init__(self, obj):
def setup_method(self, method):
pass

def test_invalida_delgation(self):
def test_invalid_delegation(self):
# these show that in order for the delegation to work
# the _delegate_* methods need to be overridden to not raise
# a TypeError
Expand Down