Series.describe returns first and last for tz-aware datetimes #21332

louispotok · 2018-06-05T21:43:25Z

GH issue 21328

closes describe() does not report first and last on tz-aware datetime Series #21328
tests added / passed
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

jreback · 2018-06-05T21:44:48Z

pandas/tests/series/test_analytics.py

@@ -336,6 +336,31 @@ def test_describe(self):
                          index=['count', 'unique', 'top', 'freq'])
        tm.assert_series_equal(result, expected)

+        start = Timestamp(2018, 1, 1)


can u add a new test and/or parametrize on the tz

codecov · 2018-06-05T22:52:53Z

Codecov Report

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

@@            Coverage Diff             @@
##           master   #21332      +/-   ##
==========================================
+ Coverage   91.93%   91.94%   +<.01%     
==========================================
  Files         159      160       +1     
  Lines       49733    49738       +5     
==========================================
+ Hits        45721    45730       +9     
+ Misses       4012     4008       -4

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

Impacted Files	Coverage Δ
pandas/core/generic.py	`96.45% <100%> (+0.24%)`	⬆️
pandas/core/arrays/categorical.py	`95.93% <0%> (-0.09%)`	⬇️
pandas/core/groupby/groupby.py	`92.65% <0%> (-0.02%)`	⬇️
pandas/core/indexes/category.py	`97.01% <0%> (ø)`	⬆️
pandas/util/testing.py	`85.33% <0%> (ø)`	⬆️
pandas/core/groupby/categorical.py	`95.45% <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 d24a950...0af4758. Read the comment docs.

mroeschke · 2018-06-06T03:43:00Z

pandas/tests/series/test_analytics.py

@@ -336,6 +336,25 @@ def test_describe(self):
                          index=['count', 'unique', 'top', 'freq'])
        tm.assert_series_equal(result, expected)

+    @pytest.mark.parametrize("tz", [None, "US/Eastern"])
+    def test_describe_dt(self, tz):
+        if tz is None:


You can put this in pytest.mark.parametrize as well.

sure, will do

mroeschke · 2018-06-06T03:48:47Z

pandas/tests/series/test_analytics.py

@@ -336,6 +336,25 @@ def test_describe(self):
                          index=['count', 'unique', 'top', 'freq'])
        tm.assert_series_equal(result, expected)

+    @pytest.mark.parametrize("tz", [None, "US/Eastern"])
+    def test_describe_dt(self, tz):


Could you make the test name a bit more descriptive, like test_describe_timezone_dtype

I don't quite think that's the right name since the test also tests tz-naive datetimes. How about test_describe_dt_dtype?

mroeschke · 2018-06-06T15:31:10Z

It would be good to add (or see if there's) a test for DataFrame.describe() with a datetime tz dtype column as well.

louispotok · 2018-06-06T16:06:52Z

I looked through the existing tests for DataFrame.describe() (in frame/test_analytics.py). It looks like they are all testing logic about include/exclude, or what happens with column names - not the core logic of what happens when each Series is passed to describe_1d. So I don't think it makes sense to add a new test there for this functionality, which is all within the describe_1d call and therefore tested by the tests on Series.

louispotok · 2018-06-12T19:05:10Z

@jreback made the suggested change, ready to merge or further changes desired?

jreback

looks good, some comments, can you add a note in 0.24.0 in bug fix section.

jreback · 2018-06-19T00:39:43Z

pandas/tests/series/test_analytics.py

@@ -336,6 +336,24 @@ def test_describe(self):
                          index=['count', 'unique', 'top', 'freq'])
        tm.assert_series_equal(result, expected)

+    @pytest.mark.parametrize("tz, name", [


can you use the tz_naive_fixture here

jreback · 2018-06-19T00:40:03Z

pandas/tests/series/test_analytics.py

+        (None, "tz-naive"),
+        ("US/Eastern", "tz-aware")
+    ])
+    def test_describe_dt(self, tz, name):


can you name: test_describe_with_tz

jreback · 2018-06-19T00:40:15Z

pandas/tests/series/test_analytics.py

+    def test_describe_dt(self, tz, name):
+        start = Timestamp(2018, 1, 1)
+        end = Timestamp(2018, 1, 5)
+        s = Series(date_range(start, end, tz=tz), name=name)


can you add a test for frame as well (in the appropraite file)

louispotok · 2018-06-19T17:58:12Z

Changes made:

add to whatsnew
use tz_naive_fixture
change name of series test

Ready for re-review

mroeschke · 2018-07-05T21:32:52Z

doc/source/whatsnew/v0.24.0.txt

@@ -131,7 +131,7 @@ Datetimelike

 - Fixed bug where two :class:`DateOffset` objects with different ``normalize`` attributes could evaluate as equal (:issue:`21404`)
 - Bug in :class:`Index` with ``datetime64[ns, tz]`` dtype that did not localize integer data correctly (:issue:`20964`)
-
+- Fixed bug where `describe` method on tz-aware datetimes did not show `first` and `last` result (:issue:`21328`)


Instead of describe, could you adjust it to :meth:DataFrame.describe and :meth:Series.describe. It will render nicer in the whatsnew doc

mroeschke · 2018-07-05T21:34:28Z

pandas/tests/frame/test_analytics.py

@@ -417,6 +417,27 @@ def test_describe_timedelta_values(self):
                    "max           5 days 00:00:00         0 days 05:00:00")
        assert repr(res) == exp_repr

+    def test_describe_tz_values(self, tz_naive_fixture):
+        tz = tz_naive_fixture


Could you add a comment with the issue number, like GH #21332

mroeschke · 2018-07-05T21:34:36Z

pandas/tests/series/test_analytics.py

@@ -336,6 +336,22 @@ def test_describe(self):
                          index=['count', 'unique', 'top', 'freq'])
        tm.assert_series_equal(result, expected)

+    def test_describe_with_tz(self, tz_naive_fixture):


Could you add a comment with the issue number, like GH #21332

mroeschke · 2018-07-05T21:36:29Z

Just a few documentation comments, one merge conflict, otherwise LGTM.

GH issue 21328

louispotok · 2018-07-06T00:54:29Z

Thanks @mroeschke . fixed.

mroeschke · 2018-07-06T14:06:18Z

doc/source/whatsnew/v0.24.0.txt

@@ -295,6 +295,8 @@ Datetimelike
 ^^^^^^^^^^^^

 - Fixed bug where two :class:`DateOffset` objects with different ``normalize`` attributes could evaluate as equal (:issue:`21404`)
+- Bug in :class:`Index` with ``datetime64[ns, tz]`` dtype that did not localize integer data correctly (:issue:`20964`)
+- Fixed bug where :meth:`DataFrame.describe` and :meth:`Series.describe` on tz-aware datetimes did not show `first` and `last` result (:issue:`21328`)


One more nit: Could you move this entry to the timezones section below?

Also the entry above this one exists in the timezone section already i.e. you can remove it.

Thanks for you patience.

Done, no problem.

jreback

@mroeschke merge when you are happy with this.

mroeschke · 2018-07-06T22:58:06Z

Thanks @louispotok!

…-dev#21332) * Series.describe returns first and last for tz-aware datetimes GH issue 21328 * parameterize tests * parameterize names * use tz_naive_fixture and fix top * add tz describe test for df * add bugfix to whatsnew * fix formatting in whatsnew and add issue number to tests * final bugfix

jreback requested changes Jun 5, 2018

View reviewed changes

mroeschke reviewed Jun 6, 2018

View reviewed changes

gfyoung added Bug Timezones Timezone data dtype labels Jun 6, 2018

jreback requested changes Jun 19, 2018

View reviewed changes

louispotok force-pushed the tz-aware-describe-21328 branch from 96ac31e to a269f5f Compare June 19, 2018 17:57

mroeschke reviewed Jul 5, 2018

View reviewed changes

louispotok added 7 commits July 5, 2018 17:51

Series.describe returns first and last for tz-aware datetimes

39f0e67

GH issue 21328

parameterize tests

be19766

parameterize names

7ddf706

use tz_naive_fixture and fix top

3eeec04

add tz describe test for df

0a9e0c6

add bugfix to whatsnew

7fe331d

fix formatting in whatsnew and add issue number to tests

dd13740

louispotok force-pushed the tz-aware-describe-21328 branch from 9a7deca to dd13740 Compare July 6, 2018 00:53

mroeschke reviewed Jul 6, 2018

View reviewed changes

jreback approved these changes Jul 6, 2018

View reviewed changes

jreback added this to the 0.24.0 milestone Jul 6, 2018

final bugfix

0af4758

mroeschke merged commit 620abc4 into pandas-dev:master Jul 6, 2018

louispotok deleted the tz-aware-describe-21328 branch July 7, 2018 01:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Series.describe returns first and last for tz-aware datetimes #21332

Series.describe returns first and last for tz-aware datetimes #21332

louispotok commented Jun 5, 2018 •

edited

Loading

jreback Jun 5, 2018

louispotok Jun 5, 2018

codecov bot commented Jun 5, 2018 •

edited

Loading

mroeschke Jun 6, 2018

louispotok Jun 6, 2018 •

edited

Loading

mroeschke Jun 6, 2018

louispotok Jun 6, 2018

mroeschke commented Jun 6, 2018

louispotok commented Jun 6, 2018

louispotok commented Jun 12, 2018

jreback left a comment •

edited

Loading

jreback Jun 19, 2018

louispotok Jun 19, 2018

jreback Jun 19, 2018

louispotok Jun 19, 2018

jreback Jun 19, 2018

louispotok Jun 19, 2018

louispotok commented Jun 19, 2018 •

edited

Loading

mroeschke Jul 5, 2018 •

edited

Loading

mroeschke Jul 5, 2018

mroeschke Jul 5, 2018

mroeschke commented Jul 5, 2018

louispotok commented Jul 6, 2018

mroeschke Jul 6, 2018

louispotok Jul 6, 2018

jreback left a comment

mroeschke commented Jul 6, 2018

Series.describe returns first and last for tz-aware datetimes #21332

Series.describe returns first and last for tz-aware datetimes #21332

Conversation

louispotok commented Jun 5, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Jun 5, 2018 • edited Loading

Codecov Report

Choose a reason for hiding this comment

louispotok Jun 6, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mroeschke commented Jun 6, 2018

louispotok commented Jun 6, 2018

louispotok commented Jun 12, 2018

jreback left a comment • edited Loading

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

louispotok commented Jun 19, 2018 • edited Loading

mroeschke Jul 5, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mroeschke commented Jul 5, 2018

louispotok commented Jul 6, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback left a comment

Choose a reason for hiding this comment

mroeschke commented Jul 6, 2018

louispotok commented Jun 5, 2018 •

edited

Loading

codecov bot commented Jun 5, 2018 •

edited

Loading

louispotok Jun 6, 2018 •

edited

Loading

jreback left a comment •

edited

Loading

louispotok commented Jun 19, 2018 •

edited

Loading

mroeschke Jul 5, 2018 •

edited

Loading