Skip to content
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

BUG: pandas Timestamp tz_localize and tz_convert do not preserve freq attribute #25247

Merged
merged 14 commits into from
Feb 11, 2019
6 changes: 3 additions & 3 deletions pandas/_libs/tslibs/timestamps.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -1187,12 +1187,12 @@ class Timestamp(_Timestamp):
value = tz_localize_to_utc(np.array([self.value], dtype='i8'), tz,
ambiguous=ambiguous,
nonexistent=nonexistent)[0]
return Timestamp(value, tz=tz)
return Timestamp(value, tz=tz, freq=self.freq)
else:
if tz is None:
# reset tz
value = tz_convert_single(self.value, UTC, self.tz)
return Timestamp(value, tz=None)
return Timestamp(value, tz=None, freq=self.freq)
else:
raise TypeError('Cannot localize tz-aware Timestamp, use '
'tz_convert for conversions')
Expand Down Expand Up @@ -1222,7 +1222,7 @@ class Timestamp(_Timestamp):
'tz_localize to localize')
else:
# Same UTC timestamp, different time zone
return Timestamp(self.value, tz=tz)
return Timestamp(self.value, tz=tz, freq=self.freq)

astimezone = tz_convert

Expand Down
9 changes: 9 additions & 0 deletions pandas/tests/tslibs/test_conversion.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,3 +66,12 @@ def test_length_zero_copy(dtype, copy):
arr = np.array([], dtype=dtype)
result = conversion.ensure_datetime64ns(arr, copy=copy)
assert result.base is (None if copy else arr)


def test_tz_convert_freq():
Copy link
Member

Choose a reason for hiding this comment

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

Can you use the tz_naive_fixture. Ensures we also test tz_localize(None) as well.

import pandas as pd
import pytz
Copy link
Member

Choose a reason for hiding this comment

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

Don't need to import this. Just the string 'UTC' will automatically use pytz.UTC

t1 = pd.Timestamp('2019-01-01 10:00', freq='H')
assert t1.tz_localize(pytz.utc).freq == t1.freq
Copy link
Member

Choose a reason for hiding this comment

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

Just assert the Timestamps are equal. IIRC the freq will be compared as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

but i assume t1.tz_localize(pytz.utc) is tz-aware while t1 is tz-naive ? @mroeschke

Copy link
Member

Choose a reason for hiding this comment

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

I guess as mentioned below. I was incorrect in assuming the freq would be compared as well.

t2 = pd.Timestamp('2019-01-02 12:00', tz=pytz.utc, freq='T')
assert t2.tz_convert(pytz.utc).freq == t2.freq
Copy link
Contributor

@nmusolino nmusolino Feb 9, 2019

Choose a reason for hiding this comment

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

These four lines look nice.

I think these tests would fit better in pandas/tests/scalar/timestamp/test_timestamp.py, which already contains tests of Timestamp.tz_localize and Timestamp.tz_convert.

One other thought: you could add a case where freq is None, or possibly add an assertion to an existing test in test_timestamp.py.