Skip to content

Fix issue #4496: tslib.tz_convert(vals, tz1, tz2) may raise an IndexErro... #4497

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.

Sign up for GitHub

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 1 commit into from
Aug 23, 2013
Merged

Fix issue #4496: tslib.tz_convert(vals, tz1, tz2) may raise an IndexErro... #4497

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions doc/source/release.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,8 @@ See :ref:`Internal Refactoring<whatsnew_0130.refactoring>`
- raising an invalid ``TypeError`` rather than ``ValueError`` when appending
with a different block ordering (:issue:`4096`)
- ``read_hdf`` was not respecting as passed ``mode`` (:issue:`4504`)
- Fixed bug in tslib.tz_convert(vals, tz1, tz2): it could raise IndexError exception while
Copy link
Contributor

Choose a reason for hiding this comment

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

can you just make this a one-liner here? (and put the explanation with the test, though could be a bit shorter)...thanks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well.. ok. Though Phillip Cloud asked me to do something that looks exact the opposite (initially my detailed explanation was located within the test doc string):

"add documentation to doc/release.rst, preferentially move the docstring in your test to release notes, but not necessary. if you just want to add a blurb that's fine too."

Anyway, I'll try to make my explanation shorter, and put a one-liner in release.rst.

Copy link
Contributor

Choose a reason for hiding this comment

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

sorry for the confusion. The idea is that release notes is a pretty short explanation, a doc-string is user visibile should shouldn't be that crazy. Docs can be full explanation. But, in this case its actually a bug fix reference (that won't be there after), so can put in the test suite.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I see. I removed detailed explanation from release.rst. Now both release.rst and doc string in unit-test reference Issue #4496, where I wrote the detailed bug description. So interested person can look there for details.

trying to access trans[pos + 1] (:issue:`4496`)
- The ``by`` argument now works correctly with the ``layout`` argument
(:issue:`4102`, :issue:`4014`) in ``*.hist`` plotting methods
- Fixed bug in ``PeriodIndex.map`` where using ``str`` would return the str
Expand Down
19 changes: 19 additions & 0 deletions pandas/tests/test_frame.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10587,6 +10587,25 @@ def test_consolidate_datetime64(self):
assert_array_equal(df.starting.values, ser_starting.index.values)
assert_array_equal(df.ending.values, ser_ending.index.values)

def test_tslib_tz_convert_trans_pos_plus_1__bug(self):
"""
Regression test for tslib.tz_convert(vals, tz1, tz2).
See https://github.com/pydata/pandas/issues/4496 for details.
"""
idx = pd.date_range(datetime(2011, 3, 26, 23), datetime(2011, 3, 27, 1), freq='1min')
idx = idx.tz_localize('UTC')
idx = idx.tz_convert('Europe/Moscow')

test_vector = pd.Series([3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3,
3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3,
3, 3, 3, 3, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4,
4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4,
4, 4, 4, 4, 4, 4, 4, 4, 5], dtype=int)

hours = idx.hour

np.testing.assert_equal(hours, test_vector.values)

def _check_bool_op(self, name, alternative, frame=None, has_skipna=True,
has_bool_only=False):
if frame is None:
Expand Down
4 changes: 3 additions & 1 deletion pandas/tslib.pyx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1432,9 +1432,11 @@ def tz_convert(ndarray[int64_t] vals, object tz1, object tz2):
pos -= 1

offset = deltas[pos]
cdef Py_ssize_t trans_len = len(trans)

for i in range(n):
v = utc_dates[i]
if v >= trans[pos + 1]:
if (pos + 1) < trans_len and v >= trans[pos + 1]:
pos += 1
offset = deltas[pos]
result[i] = v + offset
Expand Down