API/BUG: DatetimeIndex correctly localizes integer data #21216

mroeschke · 2018-05-26T05:39:36Z

closes #20997
closes #20964

tests added / passed
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

I did a little refactoring of Datetime.__new__ that converts passed data but made it easier to fix this issue. I had to modify some methods (join, normalize, delete, insert) and tests that weren't assuming integer data weren't epoch timestamps.

…t_redux

codecov · 2018-05-26T07:12:53Z

Codecov Report

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

@@            Coverage Diff             @@
##           master   #21216      +/-   ##
==========================================
- Coverage    91.9%   91.89%   -0.01%     
==========================================
  Files         153      153              
  Lines       49606    49597       -9     
==========================================
- Hits        45589    45579      -10     
- Misses       4017     4018       +1

Flag	Coverage Δ
#multiple	`90.29% <100%> (-0.01%)`	⬇️
#single	`41.88% <76%> (-0.01%)`	⬇️

Impacted Files	Coverage Δ
pandas/core/indexes/base.py	`96.62% <100%> (ø)`	⬆️
pandas/core/indexes/datetimes.py	`95.66% <100%> (-0.14%)`	⬇️

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 b5fc769...dc7e5c0. Read the comment docs.

jreback

ok looks pretty good.

can you add a whatsnew note (0.24.0)
can you test with .astype as well
can you search thru some issue for timezone / DTI. IIRC I think this might solve some (just a guess)

jreback · 2018-05-29T01:10:49Z

pandas/tests/indexes/datetimes/test_construction.py

        tm.assert_index_equal(i, i2)
        assert i.tz.zone == 'US/Eastern'

-        i2 = DatetimeIndex(i.tz_localize(None).asi8, dtype=i.dtype)


i actually like this construction, can you add a test that does both? (e.g. both uses and does not localize), maybe split it out and parameterize

jreback · 2018-05-29T01:11:13Z

pandas/tests/indexes/datetimes/test_construction.py

+    @pytest.mark.parametrize('klass', [Index, DatetimeIndex])
+    @pytest.mark.parametrize('box', [np.array, list])
+    def test_constructor_with_int_tz(self, klass, box):
+        ts = Timestamp('2018-01-01', tz='US/Pacific')


can you add the issue number

jreback · 2018-05-29T01:11:51Z

pandas/tests/indexes/test_base.py

    @pytest.mark.parametrize("klass", [pd.Index, pd.DatetimeIndex])
-    def test_constructor_dtypes_datetime(self, tz_naive_fixture, values,
+    def test_constructor_dtypes_datetime(self, tz_naive_fixture, attr, utc,
                                         klass):


can you add a bit of commentary on what is tested here

…t_redux

mroeschke · 2018-06-05T05:03:06Z

Thanks @jreback. Moved the whatsnew to v0.24, added some additional logic to get astype working. It doesn't appear to fix any of the other reported timezone issues.

jreback · 2018-06-05T10:45:39Z

pandas/core/indexes/datetimes.py

        from pandas.io.formats.format import _is_dates_only
-        return _is_dates_only(self.values)
+        return _is_dates_only(self.values) and self.tz is None


why did this need changing?

After this refactor, I was getting errors where the repr didn't look correct but the i8 data was correct in the case when a tz was passed in the DatetimeIndex constructor.

_is_dates_only tries to:

return a boolean if we are only dates (and don't have a timezone)

but checks for timezones by re-passing self.values into DatetimeIndex. I am not sure the historical reason why this was the case, but checking self.tz is None fixed the repr issue (and seems to achieve what that function was trying to do in the first place)

jreback · 2018-06-05T10:46:04Z

pandas/core/indexes/datetimes.py

        else:
            # must be integer dtype otherwise
-            if isinstance(data, Int64Index):
-                raise TypeError('cannot convert Int64Index->DatetimeIndex')
+            # assume this data are epoch timestamps
            if data.dtype != _INT64_DTYPE:
                data = data.astype(np.int64)


could
data = data.astype(np.int64, copy=False)

I don't think we have any tests with a non-int64 dtype, can you add something

jreback · 2018-06-05T10:47:01Z

pandas/tests/indexes/datetimes/test_astype.py

@@ -225,6 +225,13 @@ def _check_rng(rng):
        _check_rng(rng_eastern)
        _check_rng(rng_utc)

+    def test_integer_index_astype_datetimetz_dtype(self):


can you use tz_naive_fixture here

jreback · 2018-06-05T10:47:48Z

pandas/tests/indexes/datetimes/test_construction.py

@@ -26,25 +27,28 @@ def test_construction_caching(self):
                                                 freq='ns')})
        assert df.dttz.dtype.tz.zone == 'US/Eastern'

-    def test_construction_with_alt(self):
-
+    @pytest.mark.parametrize('kwargs', [


can you parameterize on tz_aware_fixture (doesn't include None), then parameterize as you are doing

jreback · 2018-06-05T10:48:31Z

pandas/tests/indexes/datetimes/test_construction.py

+        tm.assert_index_equal(i, result)
+        assert result.tz.zone == 'US/Eastern'
+
+    @pytest.mark.parametrize('kwargs', [


jreback · 2018-06-05T10:48:55Z

pandas/tests/indexes/datetimes/test_construction.py

+    @pytest.mark.parametrize('kwargs', [
+        {'tz': 'dtype.tz'},
+        {'dtype': 'dtype'},
+        {'dtype': 'dtype', 'tz': 'dtype.tz'}])


can you explain how this test is different than the one above?

I am preserving the construction of tz_localize(None) that was here before but split it out in its own test as discussed here #21216 (comment)

jreback · 2018-06-05T10:49:23Z

pandas/tests/indexes/datetimes/test_construction.py

@@ -469,6 +473,15 @@ def test_constructor_with_non_normalized_pytz(self, tz):
        result = DatetimeIndex(['2010'], tz=non_norm_tz)
        assert pytz.timezone(tz) is result.tz

+    @pytest.mark.parametrize('klass', [Index, DatetimeIndex])
+    @pytest.mark.parametrize('box', [np.array, list])
+    def test_constructor_with_int_tz(self, klass, box):


parameterize on tz_naive_fixture

…t_redux

jreback · 2018-06-07T11:41:17Z

looks good. @jschendel @WillAyd if you can have a look if any comments.

…t_redux

jreback · 2018-06-14T10:05:35Z

thanks @mroeschke

jreback · 2018-06-14T10:06:07Z

pandas/core/indexes/base.py

@@ -1175,6 +1175,10 @@ def astype(self, dtype, copy=True):
            return CategoricalIndex(self.values, name=self.name, dtype=dtype,
                                    copy=copy)
        try:
+            if is_datetime64tz_dtype(dtype):
+                from pandas.core.indexes.datetimes import DatetimeIndex


side issue (another PR). The imports inside functions should be as simple as possible, e.g.
from pandas import DatetimeIndex (several occurrences of this)

…1216)

mroeschke added 12 commits May 12, 2018 14:07

BUG: Localize Index with datetime dtype and integer data correctly

9626044

Adjust tests

f5235eb

Merge remote-tracking branch 'upstream/master' into index_datetime_int

82fbf0c

Adjust tests

c95ca51

Merge remote-tracking branch 'upstream/master' into index_datetime_int

af3c615

Refactor processing logic

cc68764

Merge remote-tracking branch 'upstream/master' into index_datetime_in…

ab00008

…t_redux

Adjust test

86a40a1

Merge remote-tracking branch 'upstream/master' into index_datetime_in…

069774b

…t_redux

add additional comment

74661ca

Merge remote-tracking branch 'upstream/master' into index_datetime_in…

de64718

…t_redux

adjust blank spaces

abf3efc

mroeschke added API Design Timezones Timezone data dtype labels May 26, 2018

jorisvandenbossche added this to the 0.24.0 milestone May 26, 2018

jreback requested changes May 29, 2018

View reviewed changes

mroeschke added 4 commits May 29, 2018 20:12

Merge remote-tracking branch 'upstream/master' into index_datetime_in…

3a8a714

…t_redux

Address review

ec4795b

Merge remote-tracking branch 'upstream/master' into index_datetime_in…

b71de82

…t_redux

Adjust astype

37484ea

jreback requested changes Jun 5, 2018

View reviewed changes

mroeschke added 2 commits June 6, 2018 10:33

Merge remote-tracking branch 'upstream/master' into index_datetime_in…

763a675

…t_redux

Add fixture tests, test with object data, dont copy with astype

dc0a3fe

jreback approved these changes Jun 7, 2018

View reviewed changes

mroeschke added 3 commits June 12, 2018 20:54

Merge remote-tracking branch 'upstream/master' into index_datetime_in…

2bc293d

…t_redux

Merge remote-tracking branch 'upstream/master' into index_datetime_in…

b9801a3

…t_redux

add assertion back after merge conflict

dc7e5c0

jreback merged commit 40d5982 into pandas-dev:master Jun 14, 2018

jreback reviewed Jun 14, 2018

View reviewed changes

mroeschke deleted the index_datetime_int_redux branch June 14, 2018 17:28

mroeschke mentioned this pull request Jun 15, 2018

CLN: Index imports and 0.23.1 whatsnew #21490

Merged

david-liu-brattle-1 pushed a commit to david-liu-brattle-1/pandas that referenced this pull request Jun 18, 2018

API/BUG: DatetimeIndex correctly localizes integer data (pandas-dev#2…

27f984e

…1216)

Sup3rGeo pushed a commit to Sup3rGeo/pandas that referenced this pull request Oct 1, 2018

API/BUG: DatetimeIndex correctly localizes integer data (pandas-dev#2…

79b9639

…1216)

mroeschke mentioned this pull request Jan 2, 2019

Changes to i8data for DatetimeIndex #24559

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

API/BUG: DatetimeIndex correctly localizes integer data #21216

API/BUG: DatetimeIndex correctly localizes integer data #21216

mroeschke commented May 26, 2018 •

edited by jreback

Loading

codecov bot commented May 26, 2018 •

edited

Loading

jreback left a comment

jreback May 29, 2018

jreback May 29, 2018

jreback May 29, 2018

mroeschke commented Jun 5, 2018

jreback Jun 5, 2018

mroeschke Jun 7, 2018

jreback Jun 5, 2018

jreback Jun 5, 2018

jreback Jun 5, 2018

jreback Jun 5, 2018

jreback Jun 5, 2018

jreback Jun 5, 2018

mroeschke Jun 7, 2018

jreback Jun 5, 2018

jreback commented Jun 7, 2018

jreback commented Jun 14, 2018

jreback Jun 14, 2018

API/BUG: DatetimeIndex correctly localizes integer data #21216

API/BUG: DatetimeIndex correctly localizes integer data #21216

Conversation

mroeschke commented May 26, 2018 • edited by jreback Loading

codecov bot commented May 26, 2018 • edited Loading

Codecov Report

jreback left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mroeschke commented Jun 5, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback commented Jun 7, 2018

jreback commented Jun 14, 2018

Choose a reason for hiding this comment

mroeschke commented May 26, 2018 •

edited by jreback

Loading

codecov bot commented May 26, 2018 •

edited

Loading