Skip to content

BUG: date_range linspace behavior respects tz #20988

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 2 commits into from
May 10, 2018

Conversation

mroeschke
Copy link
Member

@jschendel
Copy link
Member

jschendel commented May 9, 2018

Beat me by a few minutes!

Can you modify the test that was originally written for this behavior:

def test_date_range_convenience_periods(self):
# GH 20808
rng = date_range('2018-04-24', '2018-04-27', periods=3)
exp = DatetimeIndex(['2018-04-24 00:00:00', '2018-04-25 12:00:00',
'2018-04-27 00:00:00'], freq=None)
tm.assert_index_equal(rng, exp)
# Test if spacing remains linear if tz changes to dst in range
rng = date_range('2018-04-01 01:00:00', '2018-04-01 04:00:00',
tz='Australia/Sydney', periods=3)
exp = DatetimeIndex(['2018-04-01 01:00:00+11:00',
'2018-04-01 02:00:00+11:00',
'2018-04-01 02:00:00+10:00',
'2018-04-01 03:00:00+10:00',
'2018-04-01 04:00:00+10:00'], freq=None)

The timezone transition portion isn't being tested, as there's no tm.assert_index_equal. It also looks wrong to me; periods is set to three, but expected has five elements. I think the correct expected is:

In [2]: pd.date_range('2018-04-01 01:00:00', '2018-04-01 04:00:00', tz='Australia/Sydney', periods=3)
Out[2]:
DatetimeIndex(['2018-04-01 01:00:00+11:00', '2018-04-01 02:00:00+10:00',
               '2018-04-01 04:00:00+10:00'],
              dtype='datetime64[ns, Australia/Sydney]', freq=None)

(Note that the above is evenly spaced due to the DST transition)

Timestamp('20180103', tz='US/Eastern')]])
def test_date_range_linspacing_tz(self, start, end):
# GH 20983
result = date_range(start, end, periods=3, tz='US/Eastern')
Copy link
Member

Choose a reason for hiding this comment

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

Might be beneficial to test when start/end are tz aware but not specifying the tz param for date_range, to ensure that tz inference from start/end is okay.

@codecov
Copy link

codecov bot commented May 9, 2018

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #20988      +/-   ##
==========================================
+ Coverage   91.82%   91.82%   +<.01%     
==========================================
  Files         153      153              
  Lines       49490    49489       -1     
==========================================
+ Hits        45443    45444       +1     
+ Misses       4047     4045       -2
Flag Coverage Δ
#multiple 90.22% <100%> (ø) ⬆️
#single 41.85% <0%> (ø) ⬆️
Impacted Files Coverage Δ
pandas/core/indexes/datetimes.py 95.76% <100%> (ø) ⬆️
pandas/core/window.py 96.27% <0%> (-0.02%) ⬇️
pandas/core/indexes/base.py 96.65% <0%> (ø) ⬆️
pandas/util/testing.py 84.59% <0%> (+0.2%) ⬆️

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 b3f07b2...66a9d76. Read the comment docs.

@jreback jreback added Bug Timezones Timezone data dtype labels May 9, 2018
@jreback jreback added this to the 0.23.0 milestone May 9, 2018
Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

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

lgtm. small comment by @jschendel

@@ -587,10 +587,13 @@ def _generate(cls, start, end, periods, name, freq,
if end is not None:
end = end.tz_localize(tz).asm8
else:
# Create a linearly spaced date_range.
Copy link
Contributor

Choose a reason for hiding this comment

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

specifiy this is in local time

@mroeschke
Copy link
Member Author

Sorry for jumping the gun @jschendel! Thanks for the review and nice catch with the missing assert.

Addressed your comment as well @jreback

@jreback jreback merged commit 21dd192 into pandas-dev:master May 10, 2018
@jreback
Copy link
Contributor

jreback commented May 10, 2018

thanks @mroeschke and @jschendel

@mroeschke mroeschke deleted the date_range_linspace_tz branch May 10, 2018 15:22
topper-123 pushed a commit to topper-123/pandas that referenced this pull request May 13, 2018
topper-123 pushed a commit to topper-123/pandas that referenced this pull request May 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Timezones Timezone data dtype
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: linspace behavior for date_range is incorrect when tz aware
3 participants