-
-
Notifications
You must be signed in to change notification settings - Fork 18.2k
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
DEPR: Index.asi8 #37877
DEPR: Index.asi8 #37877
Changes from 5 commits
31dd9fd
f5bada8
bf34545
8c667da
11e8a64
3924bd2
8fa6a16
7210ec7
903252e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -415,6 +415,11 @@ def asi8(self): | |
ndarray | ||
An ndarray with int64 dtype. | ||
""" | ||
warnings.warn( | ||
"Index.asi8 is deprecated and will be removed in a future version", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can you add that the replacement is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. trouble is thats not really correct for the Index classes we're deprecating it for. For these classes .asi8 returns None There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ahh ok. are we deprecating for DTI and cousins? (I think we should do that as well to be consistent). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ill take a look; that might be pretty invasive There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok i think you should make this an internal method and deprecate the user facing one (even though we use it internally in the many parts of pandas) |
||
FutureWarning, | ||
stacklevel=2, | ||
) | ||
return None | ||
|
||
@classmethod | ||
|
@@ -4717,12 +4722,13 @@ def argsort(self, *args, **kwargs) -> np.ndarray: | |
>>> idx[order] | ||
Index(['a', 'b', 'c', 'd'], dtype='object') | ||
""" | ||
result = self.asi8 | ||
|
||
if result is None: | ||
result = np.array(self) | ||
if needs_i8_conversion(self.dtype): | ||
# TODO: these do not match the underlying EA argsort methods GH#37863 | ||
return self.asi8.argsort(*args, **kwargs) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why are you calling .asi8 here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. comment just above #37863 a handful of tests fail if we pass through to the backing EA.argsort There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. won't these show the deprecation warning? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no, because we're not deprecating it for the datetimelike classes |
||
|
||
return result.argsort(*args, **kwargs) | ||
# This works for either ndarray or EA, is overriden | ||
# by RangeIndex, MultIIndex | ||
return self._data.argsort(*args, **kwargs) | ||
|
||
@final | ||
def get_value(self, series: "Series", key): | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,6 +10,7 @@ | |
is_number, | ||
is_numeric_dtype, | ||
is_scalar, | ||
needs_i8_conversion, | ||
) | ||
from pandas.core.dtypes.generic import ABCIndexClass, ABCSeries | ||
|
||
|
@@ -123,8 +124,9 @@ def to_numeric(arg, errors="raise", downcast=None): | |
values = arg.values | ||
elif isinstance(arg, ABCIndexClass): | ||
is_index = True | ||
values = arg.asi8 | ||
if values is None: | ||
if needs_i8_conversion(arg.dtype): | ||
values = arg.asi8 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. aren't you deprecating this? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. only deprecating the one in pd.Index that returns None |
||
else: | ||
values = arg.values | ||
elif isinstance(arg, (list, tuple)): | ||
values = np.array(arg, dtype="O") | ||
|
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.
Can you clarify this deprecation is only for the non-datetimelike Index subclasses for which the attribute currently returns None?
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.
good idea, will update
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.
updated + greenish