-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
BUG: Correct Timestamp localization with tz near DST (#11481) #15934
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
Conversation
Codecov Report
@@ Coverage Diff @@
## master #15934 +/- ##
==========================================
+ Coverage 90.99% 90.99% +<.01%
==========================================
Files 145 145
Lines 49570 49570
==========================================
+ Hits 45106 45107 +1
+ Misses 4464 4463 -1
Continue to review full report at Codecov.
|
pandas/_libs/tslib.pyx
Outdated
@@ -1569,7 +1569,10 @@ cpdef convert_str_to_tsobject(object ts, object tz, object unit, | |||
ts = obj.value | |||
if tz is not None: | |||
# shift for _localize_tso | |||
ts = tz_convert_single(ts, tz, 'UTC') | |||
#ts = tz_convert_single(ts, tz, 'UTC') | |||
ts = tz_localize_to_utc(np.array([ts], dtype='i8'), tz, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm there as to be an error in tz_convert_single
. though it might simply be best to just replace that function to a call exactly like this, IOW (and see if anything else breaks)
cdef tz_convert_single(....)
return tz_localize_to_utc(.....)[0]
@@ -159,6 +159,19 @@ def test_timestamp_constructed_by_date_and_tz_explicit(self): | |||
self.assertEqual(result.hour, expected.hour) | |||
self.assertEqual(result, expected) | |||
|
|||
def test_timestamp_constructor_near_dst_boundary(self): | |||
# GH 11481 & 15777 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you give a 1 sentence on what the error was
can you scour the xref'ed issues for any more test cases and see if this closes any linked issues? (if you did already then ok). |
c03e06e
to
848c383
Compare
Added some additional tests and described the error. Addtionally, I tried dropping in |
TST: more tests, xref pandas-dev#15823, xref pandas-dev#11708
thanks @mroeschke added some more tests. will merge on pass. |
git diff upstream/master --name-only -- '*.py' | flake8 --diff
Seemed that the
Timestamp
constructor with a string incorrectly localized a naive time withtz_convert_single
instead oftz_localize_to_utc
which is similar to howDatetimeIndex
localizes.Now the
Timestamp
will raise an error if an ambiguous combination of string and timezone is passed. Should this be the default behavior?I had to slightly change 2 existing tests.
test_timestamp_to_datetime_tzoffset
used a now ambiguous timestamp andtest_setitem_with_tz_dst
had an incorrect expected timestamp.