BUG: Using DatetimeIndex.date with timezone returns incorrect date #2… #21281

jamestran201 · 2018-06-01T01:51:48Z

closes Datetime.index.date returns incorrect date post upgrade to version 0.23 #21230, closes BUG: DatetimeIndex.time incorrect in case of timezones #21267
tests added / passed
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

Added 2 tests for DatetimeIndex.date in tests/indexes/datetimes/test_datetime.py. Please let me know if I should move them somewhere else.

…1230

mroeschke · 2018-06-01T05:05:31Z

pandas/core/indexes/datetimes.py

@@ -2040,7 +2040,7 @@ def date(self):
        Returns numpy array of python datetime.date objects (namely, the date
        part of Timestamps without timezone information).
        """
-        return libts.ints_to_pydatetime(self.normalize().asi8, box="date")
+        return libts.ints_to_pydatetime(self.asi8, box="date")


I think this needs to be self._local_timestamps() instead of self.asi8 since those are essentially epoch (UTC) timestamps and we want to return local values.

Thanks for the suggestion. That might be it.

mroeschke · 2018-06-01T05:10:32Z

pandas/tests/indexes/datetimes/test_datetime.py

@@ -27,6 +27,21 @@ def test_roundtrip_pickle_with_tz(self):
        unpickled = tm.round_trip_pickle(index)
        tm.assert_index_equal(index, unpickled)

+    def test_date_accessor_with_tz(self):


Instead of 2 separate tests, you can have one test pytest.mark.parameterize over the timezone aware and naive data. (e.g. dtype='datetime64[ns, CET]'' and dtype=None)

codecov · 2018-06-02T02:19:09Z

Codecov Report

Merging #21281 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #21281      +/-   ##
==========================================
+ Coverage   91.85%   91.85%   +<.01%     
==========================================
  Files         153      153              
  Lines       49549    49555       +6     
==========================================
+ Hits        45512    45518       +6     
  Misses       4037     4037

Flag	Coverage Δ
#multiple	`90.25% <100%> (ø)`	⬆️
#single	`41.87% <0%> (-0.01%)`	⬇️

Impacted Files	Coverage Δ
pandas/core/indexes/datetimes.py	`95.8% <100%> (+0.02%)`	⬆️
pandas/core/series.py	`94.12% <0%> (ø)`	⬆️

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 0c65c57...3b15060. Read the comment docs.

jamestran201 · 2018-06-02T02:20:48Z

ASV for this commit:

[  0.00%] · For pandas commit hash d962273a:
[  0.00%] ·· Building for conda-py3.6-Cython-matplotlib-numexpr-numpy-openpyxl-pytables-pytest-scipy-sqlalchemy-xlrd-xlsxwriter-xlwt
[  0.00%] ·· Benchmarking conda-py3.6-Cython-matplotlib-numexpr-numpy-openpyxl-pytables-pytest-scipy-sqlalchemy-xlrd-xlsxwriter-xlwt
[ 50.00%] ··· Running timeseries.DatetimeIndex.time_to_date                                                                     1.23±0.1ms;...
[ 50.00%] · For pandas commit hash 4274b840:
[ 50.00%] ·· Building for conda-py3.6-Cython-matplotlib-numexpr-numpy-openpyxl-pytables-pytest-scipy-sqlalchemy-xlrd-xlsxwriter-xlwt
[ 50.00%] ·· Benchmarking conda-py3.6-Cython-matplotlib-numexpr-numpy-openpyxl-pytables-pytest-scipy-sqlalchemy-xlrd-xlsxwriter-xlwt
[100.00%] ··· Running timeseries.DatetimeIndex.time_to_date                                                                    1.20±0.08ms;...
BENCHMARKS NOT SIGNIFICANTLY CHANGED.

mroeschke · 2018-06-02T18:38:09Z

pandas/core/indexes/datetimes.py

@@ -2040,7 +2040,11 @@ def date(self):
        Returns numpy array of python datetime.date objects (namely, the date
        part of Timestamps without timezone information).
        """
-        return libts.ints_to_pydatetime(self.normalize().asi8, box="date")
+        if (self.tz is None):
+            return libts.ints_to_pydatetime(self.normalize().asi8, box="date")


Do we need the .normalize() calls here? I think ints_to_pydatetime will strip the time information away when constructing the date

mroeschke · 2018-06-02T18:38:41Z

pandas/tests/indexes/datetimes/test_datetime.py

+    ])
+    def test_date_accessor(self, test_input):
+        # GH 21230
+        from datetime import date


Looks like this is imported at the top already

mroeschke · 2018-06-02T18:39:55Z

pandas/tests/indexes/datetimes/test_datetime.py

+        # GH 21230
+        from datetime import date
+
+        assert test_input.date == np.array(date(2013, 1, 24))


Use tm.assert_numpy_array_equal here.

mroeschke · 2018-06-02T18:41:16Z

pandas/tests/indexes/datetimes/test_datetime.py

@@ -27,6 +27,18 @@ def test_roundtrip_pickle_with_tz(self):
        unpickled = tm.round_trip_pickle(index)
        tm.assert_index_equal(index, unpickled)

+    @pytest.mark.parametrize("test_input", [
+        DatetimeIndex(['2013-01-24 15:01:00']),


Instead, you could leave the DatetimeIndex construction in the test, and parametrize over the dtypes and pass that into the constructor.

jorisvandenbossche

Thanks for the PR!

Can you do a similar change for time attribute (just above date in the code) ?

jorisvandenbossche · 2018-06-04T12:28:24Z

Can you do a similar change for time attribute (just above date in the code) ?

Then it would also close #21267

jamestran201 · 2018-06-04T13:15:59Z

Sure. There's some discussion in #21267 about whether time should return a time with the timezone in it. Could you tell me which is the correct behavior?

jorisvandenbossche · 2018-06-04T13:31:13Z

I think converting back to the 0.22 behaviour is the best for now (which I think is no timezone in the output?) We can still later discuss if we want to add the timezone or not.

mroeschke · 2018-06-04T18:18:57Z

@jorisvandenbossche regarding my comment (#21267 (comment)), it looks like the time behavior looks correct but there was some confusion stemming from the construction of the DatetimeIndex (#15938).

jreback · 2018-06-04T21:20:32Z

pandas/core/indexes/datetimes.py

@@ -2040,7 +2040,11 @@ def date(self):
        Returns numpy array of python datetime.date objects (namely, the date
        part of Timestamps without timezone information).
        """
-        return libts.ints_to_pydatetime(self.normalize().asi8, box="date")
+        if (self.tz is None):


prob need
if self.tz is not None and self.tz is not utc
also write it more like

# add a comment here about tz conversions if self.tz is not None and self.tz is not utc: stamps = self.asi8 else: stamps = self._local_timestamps() return libts.init_to_pytdatetime(stamps, box="date")

jreback · 2018-06-04T21:21:34Z

pandas/tests/indexes/datetimes/test_datetime.py

+        # GH 21230
+
+        index = DatetimeIndex(['2013-01-24 15:01:00'], dtype=dtype)
+        tm.assert_numpy_array_equal(index.date,


use

result = expected =

can you add another date that is not ambiguous (e.g. its on the same date regardless of tz)
can you add a NaT as well

jreback · 2018-06-04T21:23:06Z

pandas/tests/indexes/datetimes/test_datetime.py

@@ -27,6 +27,17 @@ def test_roundtrip_pickle_with_tz(self):
        unpickled = tm.round_trip_pickle(index)
        tm.assert_index_equal(index, unpickled)

+    @pytest.mark.parametrize("dtype", [


this needs to go in test_timezones (same subdir)

place it near other tests that look at .date

jreback · 2018-06-04T21:24:39Z

doc/source/whatsnew/v0.23.1.txt

@@ -85,7 +85,7 @@ Indexing
 - Bug in :meth:`Series.reset_index` where appropriate error was not raised with an invalid level name (:issue:`20925`)
 - Bug in :func:`interval_range` when ``start``/``periods`` or ``end``/``periods`` are specified with float ``start`` or ``end`` (:issue:`21161`)
 - Bug in :meth:`MultiIndex.set_names` where error raised for a ``MultiIndex`` with ``nlevels == 1`` (:issue:`21149`)
-
+- Bug in :attr:`DatetimeIndex.date` where an incorrect date is returned when the input date has a timezone (:issue:`21230`)


this should be non-UTC timezone

…change test location

jamestran201 · 2018-06-05T02:43:08Z

@jorisvandenbossche @mroeschke @jreback For #21267, it seems that whether the timezone should be returned still needs to be discussed. Is it ok if I change the behavior of time to match v0.22 for now (i.e: time does not return a Time with timezone information) since that was also an unintended change?

jreback · 2018-06-05T02:57:55Z

wouldn’t change this as it’s correct
pls update the doc string

looking at this again -

both .date and .time should be naive i think (and .date already is)

so ok to change .time back to naive

mention in docs that these are local (naive) date/times that are returned

pls create an issue about this for discussion as well (eg to in future make these tz-aware)

jreback · 2018-06-05T02:59:25Z

pandas/tests/indexes/datetimes/test_timezones.py

+        expected = np.array([date(2018, 6, 4), pd.NaT], ndmin=1)
+
+        index = DatetimeIndex(['2018-06-04 10:00:00', pd.NaT], dtype=dtype)
+        result = index.date


can u also test .time here

jorisvandenbossche · 2018-06-05T05:34:32Z

wouldn’t change this as it’s correct

No, adding the timezone was unintended, and is inconsistent with how Timestamp itself works, and inconsistent with standard library (they both return naive local time).
So I would just change it back to how it was.

jorisvandenbossche · 2018-06-05T05:36:59Z

@jreback sorry, missed the update of your comment. So yes, then we agree!

jorisvandenbossche · 2018-06-06T07:17:38Z

pandas/tests/indexes/datetimes/test_timezones.py

+    ])
+    def test_date_accessor(self, dtype):
+        # Regression test for GH#21230
+        expected = np.array([date(2018, 6, 4), pd.NaT], ndmin=1)


is the ndmin=1 needed here?

I needed it to avoid a shape of (1,) when creating a numpy array with only 1 element. I'll remove it now.

jorisvandenbossche

minor comment, looks good to me, thanks!

jamestran201 · 2018-06-07T02:54:56Z

Is there any way to trigger a re-build on CircleCI? That build failed with this error:

CondaHTTPError: HTTP 522 ORIGIN CONNECTION TIME-OUT for url <https://repo.anaconda.com/pkgs/main/linux-64/libpq-10.3-h1ad7b7a_0.tar.bz2>
Elapsed: 00:42.034156
CF-RAY: 426f9db31db895a4-IAD

An HTTP error occurred when trying to retrieve this URL.
HTTP errors are often intermittent, and a simple retry will get you on your way.

jorisvandenbossche · 2018-06-07T09:39:36Z

@tmnhat2001 Thanks a lot!

jamestran201 · 2018-06-07T11:01:57Z

Thanks everyone.

…andas-dev#21281) * BUG: Using DatetimeIndex.date with timezone returns incorrect date pandas-dev#21230 * Fix bug where DTI.time returns a tz-aware Time instead of tz-naive pandas-dev#21267 (cherry picked from commit a363e1a)

…21281) * BUG: Using DatetimeIndex.date with timezone returns incorrect date #21230 * Fix bug where DTI.time returns a tz-aware Time instead of tz-naive #21267 (cherry picked from commit a363e1a)

…andas-dev#21281) * BUG: Using DatetimeIndex.date with timezone returns incorrect date pandas-dev#21230 * Fix bug where DTI.time returns a tz-aware Time instead of tz-naive pandas-dev#21267

BUG: Using DatetimeIndex.date with timezone returns incorrect date #2…

d27e03e

…1230

mroeschke added Datetime Datetime data dtype Timezones Timezone data dtype labels Jun 1, 2018

mroeschke reviewed Jun 1, 2018

View reviewed changes

jamestran201-alt added 2 commits June 1, 2018 20:52

Merge branch 'master' of https://github.com/pandas-dev/pandas

0abc96e

Use _local_timestamps() for tz-aware DTI and parametrize test

d962273

mroeschke reviewed Jun 2, 2018

View reviewed changes

Remove redudant code

f45a819

jorisvandenbossche added this to the 0.23.1 milestone Jun 4, 2018

jorisvandenbossche reviewed Jun 4, 2018

View reviewed changes

jreback requested changes Jun 4, 2018

View reviewed changes

jamestran201-alt added 2 commits June 4, 2018 22:14

Resolve merge conflict

d87106b

Use _local_timestamps() only for non-UTC DTI, fix documentation, and …

f09e8a4

…change test location

jreback requested changes Jun 5, 2018

View reviewed changes

jamestran201-alt added 2 commits June 5, 2018 17:59

Merge branch 'master' of https://github.com/pandas-dev/pandas

5dddd11

Fix bug where DTI.time returns a tz-aware Time instead of tz-naive

ab81279

jorisvandenbossche reviewed Jun 6, 2018

View reviewed changes

jorisvandenbossche approved these changes Jun 6, 2018

View reviewed changes

jamestran201-alt added 2 commits June 6, 2018 21:16

Merge branch 'master' of https://github.com/pandas-dev/pandas

2ead698

Remove redundant code

3b15060

jorisvandenbossche merged commit a363e1a into pandas-dev:master Jun 7, 2018

mroeschke mentioned this pull request Jun 7, 2018

ENH: Add timetz attribute to DatetimeIndex and Timestamp #21358

Closed

TomAugspurger added the Needs Backport label Jun 12, 2018

TomAugspurger removed the Needs Backport label Jun 12, 2018

Liam3851 mentioned this pull request Jun 19, 2018

DatetimeProperties.date problem with time zones #21542

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BUG: Using DatetimeIndex.date with timezone returns incorrect date #2… #21281

BUG: Using DatetimeIndex.date with timezone returns incorrect date #2… #21281

jamestran201 commented Jun 1, 2018 •

edited by jorisvandenbossche

Loading

mroeschke Jun 1, 2018

jamestran201 Jun 2, 2018

mroeschke Jun 1, 2018

codecov bot commented Jun 2, 2018 •

edited

Loading

jamestran201 commented Jun 2, 2018

mroeschke Jun 2, 2018

mroeschke Jun 2, 2018

mroeschke Jun 2, 2018

mroeschke Jun 2, 2018

jorisvandenbossche left a comment

jorisvandenbossche commented Jun 4, 2018

jamestran201 commented Jun 4, 2018

jorisvandenbossche commented Jun 4, 2018

mroeschke commented Jun 4, 2018

jreback Jun 4, 2018

jreback Jun 4, 2018

jreback Jun 4, 2018

jreback Jun 4, 2018

jreback Jun 4, 2018

jreback Jun 4, 2018

jamestran201 commented Jun 5, 2018

jreback commented Jun 5, 2018 •

edited

Loading

jreback Jun 5, 2018

jorisvandenbossche commented Jun 5, 2018

jorisvandenbossche commented Jun 5, 2018

jorisvandenbossche Jun 6, 2018

jamestran201 Jun 7, 2018

jorisvandenbossche left a comment

jamestran201 commented Jun 7, 2018

jorisvandenbossche commented Jun 7, 2018

jamestran201 commented Jun 7, 2018

BUG: Using DatetimeIndex.date with timezone returns incorrect date #2… #21281

BUG: Using DatetimeIndex.date with timezone returns incorrect date #2… #21281

Conversation

jamestran201 commented Jun 1, 2018 • edited by jorisvandenbossche Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Jun 2, 2018 • edited Loading

Codecov Report

jamestran201 commented Jun 2, 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

jorisvandenbossche left a comment

Choose a reason for hiding this comment

jorisvandenbossche commented Jun 4, 2018

jamestran201 commented Jun 4, 2018

jorisvandenbossche commented Jun 4, 2018

mroeschke commented Jun 4, 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

jamestran201 commented Jun 5, 2018

jreback commented Jun 5, 2018 • edited Loading

Choose a reason for hiding this comment

jorisvandenbossche commented Jun 5, 2018

jorisvandenbossche commented Jun 5, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jorisvandenbossche left a comment

Choose a reason for hiding this comment

jamestran201 commented Jun 7, 2018

jorisvandenbossche commented Jun 7, 2018

jamestran201 commented Jun 7, 2018

jamestran201 commented Jun 1, 2018 •

edited by jorisvandenbossche

Loading

codecov bot commented Jun 2, 2018 •

edited

Loading

jreback commented Jun 5, 2018 •

edited

Loading