ENH: Support TZ Aware IntervalIndex #18558

jschendel · 2017-11-29T04:48:12Z

closes ENH: Add full tz aware support for IntervalIndex #18537
closes BUG: Interval can contain end points with different time zones #18538
tests added / passed
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

Updates to Interval:

Disallowed Interval objects with mixed time zones
Minor docstring updates

Updates to IntervalIndex:

Disallowed IntervalIndex with mixed time zones
Fixed IntervalIndex.mid for tz aware
Fixed _get_next_label and _get_previous_label for tz aware
- cascades to other methods
Cleaned up error message for mixed left/right types

jbrockmendel · 2017-11-29T05:27:41Z

pandas/_libs/interval.pyx

+            msg = ("left and right must have the same time zone, got "
+                   "'{left_tz}' and '{right_tz}'").format(
+                   left_tz=left.tzinfo, right_tz=right.tzinfo)
+            raise ValueError(msg)


What about pytz tzinfos? Eg US/Eastern is represented by a different tzinfo object for DST and non-DST

Does my switch to using get_timezone fully address this? The docstring seems to indicate so, but I'm not super familiar with this part of the codebase.

I think that should do it, thanks.

codecov · 2017-11-29T07:36:27Z

Codecov Report

Merging #18558 into master will decrease coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #18558      +/-   ##
==========================================
- Coverage   91.35%   91.34%   -0.01%     
==========================================
  Files         164      164              
  Lines       49802    49813      +11     
==========================================
+ Hits        45496    45502       +6     
- Misses       4306     4311       +5

Flag	Coverage Δ
#multiple	`89.14% <100%> (+0.01%)`	⬆️
#single	`40.8% <25%> (-0.07%)`	⬇️

Impacted Files	Coverage Δ
pandas/core/indexes/interval.py	`94.02% <100%> (+0.8%)`	⬆️
pandas/io/gbq.py	`25% <0%> (-58.34%)`	⬇️
pandas/core/frame.py	`97.81% <0%> (-0.1%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 32f562d...2311143. Read the comment docs.

codecov · 2017-11-29T07:36:28Z

Codecov Report

Merging #18558 into master will decrease coverage by 0.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #18558      +/-   ##
==========================================
- Coverage   91.59%   91.58%   -0.02%     
==========================================
  Files         153      153              
  Lines       51212    51217       +5     
==========================================
- Hits        46908    46906       -2     
- Misses       4304     4311       +7

Flag	Coverage Δ
#multiple	`89.44% <100%> (ø)`	⬆️
#single	`40.67% <28.57%> (-0.11%)`	⬇️

Impacted Files	Coverage Δ
pandas/core/indexes/interval.py	`93.8% <100%> (+0.74%)`	⬆️
pandas/io/gbq.py	`25% <0%> (-58.34%)`	⬇️
pandas/util/testing.py	`81.82% <0%> (-0.2%)`	⬇️
pandas/core/frame.py	`97.81% <0%> (-0.1%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 695e893...79e255c. Read the comment docs.

jschendel · 2017-11-29T07:54:25Z

pandas/core/indexes/interval.py

+            # handle tz aware
+            if tz:
+                data = self.left.tz_localize(None) + 0.5 * delta
+                data = data.tz_localize(tz)


Is there a better way to do this computation? Couldn't get the correct result without removing the time zone, then adding it back afterwards.

Trying to get the midpoint between elements in left and right. Basic setup is along the lines of:

In [2]: left = pd.date_range('2017-01-01', periods=3, tz='US/Eastern') ...: right = pd.date_range('2017-01-02', periods=3, tz='US/Eastern') ...: delta = right.values - left.values ...:

The existing method removed the tz, and had the wrong time (all times should be 12:00):

In [3]: left.values + 0.5 * delta Out[3]: array(['2017-01-01T17:00:00.000000000', '2017-01-02T17:00:00.000000000', '2017-01-03T17:00:00.000000000'], dtype='datetime64[ns]')

Operating on the index itself instead of the values keeps the tz, but the time is still wrong:

In [4]: left + 0.5 * delta Out[4]: DatetimeIndex(['2017-01-01 17:00:00-05:00', '2017-01-02 17:00:00-05:00', '2017-01-03 17:00:00-05:00'], dtype='datetime64[ns, US/Eastern]', freq='D')

Having delta as a TimdeltaIndex raises:

In [20]: left + 0.5 * (right - left) --------------------------------------------------------------------------- TypeError: data type not understood

[20] should work, we might have an issue about this; let's fix this fix this first in another PR. don't use .values with a datetime generally, you either want to operate on the .asi8, the actual values (generally you will also need to .tz_localize(None) first to make them UTC, then reverse this later though).

Ah, yes, looks like #17558 is the issue for this, and #17583 is an open PR, albeit somewhat stale.

ok I closed that other PR as its stale. So options are to: issue a new PR before this one to fix #17558, or to do hacky fix here with a TODO, or xfail the tests for this (for now).

I'm planning to look into that issue over the next few days and open new PR to fix it prior to this one being merged.

jreback · 2017-11-29T11:43:13Z

pandas/core/indexes/interval.py

+        elif isinstance(left, ABCPeriodIndex):
+            msg = 'Period dtypes are not supported, use a PeriodIndex instead'
+            raise ValueError(msg)
+        elif isinstance(left, ABCDatetimeIndex) and left.tz != right.tz:


we usually compare
str(left.tz) != str(right.tz)

jreback · 2017-11-29T11:46:09Z

pandas/core/indexes/interval.py

+            # handle tz aware
+            if tz:
+                data = self.left.tz_localize(None) + 0.5 * delta
+                data = data.tz_localize(tz)


[20] should work, we might have an issue about this; let's fix this fix this first in another PR. don't use .values with a datetime generally, you either want to operate on the .asi8, the actual values (generally you will also need to .tz_localize(None) first to make them UTC, then reverse this later though).

jreback · 2017-11-29T11:46:36Z

pandas/core/indexes/interval.py

+            else:
+                data = self.left + 0.5 * delta
+
+            return DatetimeIndex(data, freq=freq, tz=tz)


you shouldn't need to reconvert the type

jschendel · 2017-12-07T08:00:15Z

Fixed the mid computation now that #18653 is merged. I believe that was the last item that was currently outstanding here.

jreback · 2017-12-08T11:26:17Z

thanks @jschendel

jbrockmendel reviewed Nov 29, 2017

View reviewed changes

jschendel force-pushed the iv-idx-tz-aware branch from bda1567 to 2311143 Compare November 29, 2017 07:36

jschendel commented Nov 29, 2017

View reviewed changes

jreback added Interval Interval data type Timezones Timezone data dtype labels Nov 29, 2017

jreback requested changes Nov 29, 2017

View reviewed changes

jschendel force-pushed the iv-idx-tz-aware branch from 2311143 to 8ff02ef Compare December 1, 2017 05:16

jschendel mentioned this pull request Dec 6, 2017

BUG: Fix tz-aware DatetimeIndex +/- TimedeltaIndex/timedelta64 array #18653

Merged

4 tasks

jschendel added 3 commits December 7, 2017 00:14

BUG: Support IntervalIndex TZ Aware

3af3aa4

initial fixes

070873d

fix mid computation after bug fix

79e255c

jschendel force-pushed the iv-idx-tz-aware branch from 8ff02ef to 79e255c Compare December 7, 2017 07:32

jreback added this to the 0.22.0 milestone Dec 8, 2017

jreback approved these changes Dec 8, 2017

View reviewed changes

jreback merged commit 0229538 into pandas-dev:master Dec 8, 2017

jreback mentioned this pull request Dec 8, 2017

Add test for GH 18523 and add _tz_compare() to compare timezone of DatetimeIndex #18596

Closed

4 tasks

jschendel deleted the iv-idx-tz-aware branch December 8, 2017 15:47

Uh oh!

ENH: Support TZ Aware IntervalIndex #18558

ENH: Support TZ Aware IntervalIndex #18558

Uh oh!

Conversation

jschendel commented Nov 29, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jschendel Nov 29, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Nov 29, 2017

Codecov Report

Uh oh!

codecov bot commented Nov 29, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jschendel commented Dec 7, 2017

Uh oh!

jreback commented Dec 8, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jschendel commented Nov 29, 2017 •

edited

Loading

jschendel Nov 29, 2017 •

edited

Loading

codecov bot commented Nov 29, 2017 •

edited

Loading