-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
DEPR: removal of deprecation warnings for float indexers #12246
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
Changes from all commits
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 |
---|---|---|
|
@@ -115,7 +115,11 @@ def _get_setitem_indexer(self, key): | |
|
||
try: | ||
return self._convert_to_indexer(key, is_setter=True) | ||
except TypeError: | ||
except TypeError as e: | ||
|
||
# invalid indexer type vs 'other' indexing errors | ||
if 'cannot do' in str(e): | ||
raise | ||
raise IndexingError(key) | ||
|
||
def __setitem__(self, key, value): | ||
|
@@ -312,6 +316,18 @@ def _setitem_with_indexer(self, indexer, value): | |
index = self.obj.index | ||
new_index = index.insert(len(index), indexer) | ||
|
||
# we have a coerced indexer, e.g. a float | ||
# that matches in an Int64Index, so | ||
# we will not create a duplicate index, rather | ||
# index to that element | ||
# e.g. 0.0 -> 0 | ||
# GH12246 | ||
if index.is_unique: | ||
new_indexer = index.get_indexer([new_index[-1]]) | ||
if (new_indexer != -1).any(): | ||
return self._setitem_with_indexer(new_indexer, | ||
value) | ||
|
||
# this preserves dtype of the value | ||
new_values = Series([value])._values | ||
if len(self.obj._values): | ||
|
@@ -1091,8 +1107,17 @@ def _convert_to_indexer(self, obj, axis=0, is_setter=False): | |
""" | ||
labels = self.obj._get_axis(axis) | ||
|
||
# if we are a scalar indexer and not type correct raise | ||
obj = self._convert_scalar_indexer(obj, axis) | ||
if isinstance(obj, slice): | ||
return self._convert_slice_indexer(obj, axis) | ||
|
||
# try to find out correct indexer, if not type correct raise | ||
try: | ||
obj = self._convert_scalar_indexer(obj, axis) | ||
except TypeError: | ||
|
||
# but we will allow setting | ||
if is_setter: | ||
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. Does this mean that 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. short answer is yes, though, 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. Hmm, this seems not great (on your branch) In [10]: s = Series([1,2,3])
In [11]: s[1.] = 5
In [12]: s
Out[12]:
0 1
1 2
2 3
1 5
dtype: int64
In [13]: s.index
Out[13]: Int64Index([0, 1, 2, 1], dtype='int64') So before this would have warned about the Floating scalar, cast to an int, and then overwritten the value at I think I see why you would want to allow setting of floats that could be cast as integers, but maybe you could explain your thinking a bit more? EDIT: I accidentally left out a "not" in my first line, so now I sound like a sarcastic jerk 😄 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. So I was fixing this bug
so but I couldn't get it to work and it seemed innocuous enough. Let me have another look. This is a giant rabbit hole. 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.
This is on current master. This should work and coerce to a
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. So I fixed everything, but had to change this: What kind of index should this be: w/o actually running it? |
||
pass | ||
|
||
# see if we are positional in nature | ||
is_int_index = labels.is_integer() | ||
|
@@ -1131,10 +1156,7 @@ def _convert_to_indexer(self, obj, axis=0, is_setter=False): | |
|
||
return obj | ||
|
||
if isinstance(obj, slice): | ||
return self._convert_slice_indexer(obj, axis) | ||
|
||
elif is_nested_tuple(obj, labels): | ||
if is_nested_tuple(obj, labels): | ||
return labels.get_locs(obj) | ||
elif is_list_like_indexer(obj): | ||
if is_bool_indexer(obj): | ||
|
@@ -1278,7 +1300,7 @@ def _get_slice_axis(self, slice_obj, axis=0): | |
|
||
labels = obj._get_axis(axis) | ||
indexer = labels.slice_indexer(slice_obj.start, slice_obj.stop, | ||
slice_obj.step) | ||
slice_obj.step, kind=self.name) | ||
|
||
if isinstance(indexer, slice): | ||
return self._slice(indexer, axis=axis, kind='iloc') | ||
|
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.
maybe mention that it only is about positional indexing? Otherwise people could think FloatIndex is removed.