-
-
Notifications
You must be signed in to change notification settings - Fork 17.7k
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
BUG: is ser[:2] with Int64Index positional or label-based #49612
Comments
@jbrockmendel your logic seems correct here. agree that s[:2] should be treated as location and warn that it's not positional |
@mroeschke thoughts on what to do here? |
I know you wanted to focus on int indexes, but It would be nice is this was positional to align with Unless you had come across this and wanted to change this too |
IIUC what you're suggesting would make/keep int-slicing-with-intindex inconsistent with other int-indexing-with-intindex, but possibly in a way that has fewer corner cases than the status quo? (Long-term (i.e. 3.0) im increasingly thinking we should just deprecate |
True,
+10. |
So I guess for 2.0 we just revert the deprecation? |
Probably good to get more thoughts from others who have looked closer at indexing behavior @phofl @jorisvandenbossche |
Actually just reverting wouldn't quite get to the consistent-ish behavior you suggested. we'd also need to change the behavior with Float64Index, which ATM always treats slicing as label-based. Doing that breaks 5 tests locally, though only one directly testing this behavior. |
Focusing only on Integer-dtype indexes, i.e. Int64Index, UInt64Index, RangeIndex, and NumericIndex[inty].
ser[2]
andser[[2]]
each treat the indexing as label-based instead of positional. Butser[:2]
treats the slice as positional instead of label-based.We've deprecated some of these cases in #45162, but as I go to enforce the deprecation I think I screwed up and issued the warning for too few cases. In particular, to keep the number of warnings down I excluded some cases
The
elif start is None and stop is None
case is correct, but the other two are wrong bc with.loc
slicing is right-inclusive.If I enforce the deprecation for these extra two cases, I get 555 test failures, so this isn't trivial.
The text was updated successfully, but these errors were encountered: