-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
Avoid Index DeprecationWarning in Series getitem #31361
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
Avoid Index DeprecationWarning in Series getitem #31361
Conversation
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.
Thanks, looks good!
Oh, I got who calls what backwards here... Will need to re-think things... We don't want to change all the index subclasses |
Changed to using a warnings filter. Note that this has a perf impact for slicing a Series. ~25% slower Master
This PR
Is that slowdown acceptable? Right now, I don't think it is, given that
|
I think there are 2 ways we get to get_slice with non-slice. One of them is L931, and can be avoided be extracting the single-item list after potentially warning in #31333. The other is on L940 and is explicltly an "mpl hackaround", so we can maybe-unpack there. |
Thanks @jbrockmendel, that did it. We're able to apply the filter only when using the mpl compat path. master In [2]: s = pd.Series(range(10000))
In [3]: %timeit s[:, None]
36.2 µs ± 293 ns per loop (mean ± std. dev. of 7 runs, 10000 loops each)
In [4]: t = pd.Series([], dtype=float)
In [5]: %timeit t[:]
52 µs ± 3.5 µs per loop (mean ± std. dev. of 7 runs, 10000 loops each)
In [6]: %timeit s[:5]
45 µs ± 2.27 µs per loop (mean ± std. dev. of 7 runs, 10000 loops each) PR In [4]: s = pd.Series(range(10000))
In [5]: %timeit s[:, None]
50.2 µs ± 2.56 µs per loop (mean ± std. dev. of 7 runs, 10000 loops each)
In [6]: t = pd.Series([], dtype=float)
In [7]: %timeit t[:]
48.4 µs ± 2 µs per loop (mean ± std. dev. of 7 runs, 10000 loops each)
In [8]: %timeit s[:5]
44.3 µs ± 2.52 µs per loop (mean ± std. dev. of 7 runs, 10000 loops each) So the first case is the only one with a slowdown, which I think is acceptable given that we want to deprecate that behavior anyway. |
Had to xfail a test asserting that this produced a warning. Will want to restore that with one from |
@@ -1505,7 +1505,7 @@ def get_slice(self, slobj, axis=0): | |||
if axis >= self.ndim: | |||
raise IndexError("Requested axis not found in manager") | |||
|
|||
return type(self)(self._block._slice(slobj), self.index[slobj], fastpath=True) | |||
return type(self)(self._block._slice(slobj), self.index[slobj], fastpath=True,) |
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.
i think we dont want the trailing comma here
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.
Worth rerunning CI over?
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.
i can change it in my next "assorted cleanups" PR
Sounds good |
…31403) Co-authored-by: Tom Augspurger <[email protected]>
xref #30867
cc @jorisvandenbossche. The alternative is to use a warnings filter inside SingleBlockManger.get_slice to filter the warning, but I think avoiding the warning in the first place is a bit nicer.