Bug: Allow np.timedelta64 objects to index TimedeltaIndex #20408
Conversation
| @@ -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)): | |||
There was a problem hiding this comment.
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: objectThis doesn't seem like the intended behavior, and is_number returns True for booleans.
There was a problem hiding this comment.
is_number is pretty general, technically a bool is a number (as it derives from int, as does np.timedelta).
There was a problem hiding this comment.
is_numberis 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.
There was a problem hiding this comment.
Yes, this is a gotcha. See numpy/numpy#10685 for the upstream numpy issue.
There was a problem hiding this comment.
@jschendel you make a good point
can u open an issue (or PR!)
There was a problem hiding this comment.
@jreback : Will do. Looking into this now more generally across various types of indexes.
Codecov Report
@@ Coverage Diff @@
## master #20408 +/- ##
=======================================
Coverage 91.8% 91.8%
=======================================
Files 152 152
Lines 49205 49205
=======================================
Hits 45171 45171
Misses 4034 4034
Continue to review full report at Codecov.
|
jreback
added
Bug
Indexing
|
thanks @mroeschke |
|
thanks! |
…ame_describe * upstream/master: (158 commits) Add link to "Craft Minimal Bug Report" blogpost (pandas-dev#20431) BUG: fixed json_normalize for subrecords with NoneTypes (pandas-dev#20030) (pandas-dev#20399) BUG: ExtensionArray.fillna for scalar values (pandas-dev#20412) DOC" update the Pandas core window rolling count docstring" (pandas-dev#20264) DOC: update the pandas.DataFrame.plot.hist docstring (pandas-dev#20155) DOC: Only use ~ in class links to hide prefixes. (pandas-dev#20402) Bug: Allow np.timedelta64 objects to index TimedeltaIndex (pandas-dev#20408) DOC: add disallowing of Series construction of len-1 list with index to whatsnew (pandas-dev#20392) MAINT: Remove weird pd file DOC: update the Index.isin docstring (pandas-dev#20249) BUG: Handle all-NA blocks in concat (pandas-dev#20382) DOC: update the pandas.core.resample.Resampler.fillna docstring (pandas-dev#20379) BUG: Don't raise exceptions splitting a blank string (pandas-dev#20067) DOC: update the pandas.DataFrame.cummax docstring (pandas-dev#20336) DOC: update the pandas.core.window.x.mean docstring (pandas-dev#20265) DOC: update the api.types.is_number docstring (pandas-dev#20196) Fix linter (pandas-dev#20389) DOC: Improved the docstring of pandas.Series.dt.to_pytimedelta (pandas-dev#20142) DOC: update the pandas.Series.dt.is_month_end docstring (pandas-dev#20181) DOC: update the window.Rolling.min docstring (pandas-dev#20263) ...
git diff upstream/master -u -- "*.py" | flake8 --diffis_integer(label)would catchnp.timedelta64objects due to numpy/numpy#10685 (I believe), so ensure that the label is also not a timedelta64 dtype