-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
DEPR: deprecate Index.__getitem__ with float key #34193
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: deprecate Index.__getitem__ with float key #34193
Conversation
looks good, does this close #34191? can you add a linkin the deprecations removal issue (2.0) |
@@ -1188,6 +1189,8 @@ def _post_plot_logic(self, ax, data): | |||
from matplotlib.ticker import FixedLocator | |||
|
|||
def get_label(i): | |||
if is_float(i) and i.is_integer(): | |||
i = int(i) |
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 am not really sure this is correct, but it's what is currently actually is being done (the conversion to int is just happening inside the Index.__getitem__
). So this change is needed to preserve the current behaviour.
It's a bit strange that those xticks are floats, though. But maybe that's just a matplotlib behaviour.
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.
this is really strange ,what is i 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.
It are the values from ax.get_xticks()
, see 5 lines below. I suppose matplotlib just always returns floats
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.
Yes, just checked, matplotlib simply always returns floats. And it's only for the "integer floats" that we have a value in the index to put as xtick label.
@@ -586,6 +586,9 @@ Deprecations | |||
- :func:`pandas.api.types.is_categorical` is deprecated and will be removed in a future version; use `:func:pandas.api.types.is_categorical_dtype` instead (:issue:`33385`) | |||
- :meth:`Index.get_value` is deprecated and will be removed in a future version (:issue:`19728`) | |||
- :meth:`DateOffset.__call__` is deprecated and will be removed in a future version, use ``offset + other`` instead (:issue:`34171`) | |||
- Indexing an :class:`Index` object with a float key is deprecated, and will | |||
raise an IndexError in the future. You can manually convert to an integer key | |||
instead (:issue:`34191`). |
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.
usually we dont have a trailing period. double ticks on IndexError?
pandas/core/common.py
Outdated
""" | ||
To avoid numpy DeprecationWarnings, cast float to integer where valid. | ||
|
||
Parameters | ||
---------- | ||
val : scalar | ||
warn_float : bool, default False | ||
If True, raise deprecation warning for a float indexer. |
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.
"raise" -> "issue"
|
||
@pytest.mark.parametrize( | ||
"idx", [Index([1, 2, 3]), Index([0.1, 0.2, 0.3]), Index(["a", "b", "c"])] | ||
) |
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.
could use indices fixture?
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 don't think so, as the deprecation is only for those 3 index types
few comments, generally looks good |
Added |
See #34191 for motivation
Closes #34191