Skip to content

Bug: Allow np.timedelta64 objects to index TimedeltaIndex #20408

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 3 commits into from
Mar 19, 2018
Merged

Bug: Allow np.timedelta64 objects to index TimedeltaIndex #20408

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
139a976
Bug: Allow np.timedelta64 objects to index TimedeltaIndex
mroeschke Mar 19, 2018
15a393f
rephrase whatsnew and remove newline
mroeschke Mar 19, 2018
199e819
docs
jreback Mar 19, 2018
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
3 changes: 2 additions & 1 deletion doc/source/whatsnew/v0.23.0.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -887,7 +887,8 @@ Timedelta
- Bug in :func:`Timedelta.total_seconds()` causing precision errors i.e. ``Timedelta('30S').total_seconds()==30.000000000000004`` (:issue:`19458`)
- Bug in :func: `Timedelta.__rmod__` where operating with a ``numpy.timedelta64`` returned a ``timedelta64`` object instead of a ``Timedelta`` (:issue:`19820`)
- Multiplication of :class:`TimedeltaIndex` by ``TimedeltaIndex`` will now raise ``TypeError`` instead of raising ``ValueError`` in cases of length mis-match (:issue`19333`)
-
- Bug in indexing a :class:`TimedeltaIndex` with a ``np.timedelta64`` object which was raising a ``TypeError`` (:issue:`20393`)


Timezones
^^^^^^^^^
Expand Down
3 changes: 2 additions & 1 deletion pandas/core/indexes/timedeltas.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -829,7 +829,8 @@ def _maybe_cast_slice_bound(self, label, side, kind):
else:
return (lbound + to_offset(parsed.resolution) -
Timedelta(1, 'ns'))
elif is_integer(label) or is_float(label):
elif ((is_integer(label) or is_float(label)) and
not is_timedelta64_dtype(label)):
Copy link
Member

Choose a reason for hiding this comment

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

Would using something like is_number(label) and not is_timedelta64_dtype(label) be more appropriate here?

Right now it looks like a few things can sneak through, e.g. booleans:

In [2]: s = pd.Series(list('abcde'), pd.timedelta_range(0, 4, freq='ns'))

In [3]: s
Out[3]:
00:00:00           a
00:00:00.000000    b
00:00:00.000000    c
00:00:00.000000    d
00:00:00.000000    e
Freq: N, dtype: object

In [4]: s.loc[False:True]
Out[4]:
00:00:00           a
00:00:00.000000    b
Freq: N, dtype: object

This doesn't seem like the intended behavior, and is_number returns True for booleans.

Copy link
Contributor

Choose a reason for hiding this comment

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

is_number is pretty general, technically a bool is a number (as it derives from int, as does np.timedelta).

Copy link
Member

Choose a reason for hiding this comment

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

is_number is pretty general

Yes, this was the point I was trying to make. The current approach here doesn't look general enough, as things like booleans are still allowed, as per my example. Seems like using is_number(label) and not is_timedelta64_dtype(label) would catch this appropriately.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, this is a gotcha. See numpy/numpy#10685 for the upstream numpy issue.

Copy link
Contributor

Choose a reason for hiding this comment

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

@jschendel you make a good point

can u open an issue (or PR!)

Copy link
Member

Choose a reason for hiding this comment

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

@jreback : Will do. Looking into this now more generally across various types of indexes.

self._invalid_indexer('slice', label)

return label
Expand Down
12 changes: 12 additions & 0 deletions pandas/tests/indexing/test_timedelta.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,3 +68,15 @@ def test_listlike_setitem(self, value):
series.iloc[0] = value
expected = pd.Series([pd.NaT, 1, 2], dtype='timedelta64[ns]')
tm.assert_series_equal(series, expected)

@pytest.mark.parametrize('start,stop, expected_slice', [
[np.timedelta64(0, 'ns'), None, slice(0, 11)],
[np.timedelta64(1, 'D'), np.timedelta64(6, 'D'), slice(1, 7)],
[None, np.timedelta64(4, 'D'), slice(0, 5)]])
def test_numpy_timedelta_scalar_indexing(self, start, stop,
expected_slice):
# GH 20393
s = pd.Series(range(11), pd.timedelta_range('0 days', '10 days'))
result = s.loc[slice(start, stop)]
expected = s.iloc[expected_slice]
tm.assert_series_equal(result, expected)