-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
CLN: trim unreachable indexing code #31768
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 4 commits
1b71ab9
102ce46
66669fd
57000ed
398377a
35fe788
0765ffb
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 |
---|---|---|
|
@@ -902,7 +902,7 @@ def _get_with(self, key): | |
return self._get_values(key) | ||
raise | ||
|
||
if not isinstance(key, (list, np.ndarray, Series, Index)): | ||
if not isinstance(key, (list, np.ndarray, ExtensionArray, Series, Index)): | ||
key = list(key) | ||
|
||
if isinstance(key, Index): | ||
|
@@ -987,8 +987,6 @@ def __setitem__(self, key, value): | |
|
||
try: | ||
self._set_with_engine(key, value) | ||
except com.SettingWithCopyError: | ||
raise | ||
except (KeyError, ValueError): | ||
values = self._values | ||
if is_integer(key) and not self.index.inferred_type == "integer": | ||
|
@@ -997,9 +995,6 @@ def __setitem__(self, key, value): | |
self[:] = value | ||
else: | ||
self.loc[key] = value | ||
except InvalidIndexError: | ||
# e.g. slice | ||
self._set_with(key, value) | ||
|
||
except TypeError as e: | ||
if isinstance(key, tuple) and not isinstance(self.index, MultiIndex): | ||
|
@@ -1070,7 +1065,7 @@ def _set_with(self, key, value): | |
|
||
def _set_labels(self, key, value): | ||
key = com.asarray_tuplesafe(key) | ||
indexer = self.index.get_indexer(key) | ||
indexer: np.ndarray = self.index.get_indexer(key) | ||
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. Shouldn't need to annotate like this; if anything should set the return of 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. yah, im trying to pin down what gets passed to _set_values below. The check on L1075 is never True, so id like to see if we can rip it out |
||
mask = indexer == -1 | ||
if mask.any(): | ||
raise ValueError(f"{key[mask]} not contained in the index") | ||
|
@@ -1096,12 +1091,6 @@ def _set_value(self, label, value, takeable: bool = False): | |
value : object | ||
Scalar value. | ||
takeable : interpret the index as indexers, default False | ||
|
||
Returns | ||
------- | ||
Series | ||
If label is contained, will be reference to calling Series, | ||
otherwise a new object. | ||
""" | ||
try: | ||
if takeable: | ||
|
@@ -1115,8 +1104,6 @@ def _set_value(self, label, value, takeable: bool = False): | |
# set using a non-recursive method | ||
self.loc[label] = value | ||
|
||
return self | ||
|
||
# ---------------------------------------------------------------------- | ||
# Unsorted | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1369,29 +1369,28 @@ def test_set_value(self, float_frame): | |
|
||
def test_set_value_resize(self, float_frame): | ||
|
||
res = float_frame._set_value("foobar", "B", 0) | ||
assert res is float_frame | ||
assert res.index[-1] == "foobar" | ||
assert res._get_value("foobar", "B") == 0 | ||
float_frame._set_value("foobar", "B", 0) | ||
assert float_frame.index[-1] == "foobar" | ||
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. you could assert the return value is None |
||
assert float_frame._get_value("foobar", "B") == 0 | ||
|
||
float_frame.loc["foobar", "qux"] = 0 | ||
assert float_frame._get_value("foobar", "qux") == 0 | ||
|
||
res = float_frame.copy() | ||
res3 = res._set_value("foobar", "baz", "sam") | ||
assert res3["baz"].dtype == np.object_ | ||
res._set_value("foobar", "baz", "sam") | ||
assert res["baz"].dtype == np.object_ | ||
|
||
res = float_frame.copy() | ||
res3 = res._set_value("foobar", "baz", True) | ||
assert res3["baz"].dtype == np.object_ | ||
res._set_value("foobar", "baz", True) | ||
assert res["baz"].dtype == np.object_ | ||
|
||
res = float_frame.copy() | ||
res3 = res._set_value("foobar", "baz", 5) | ||
assert is_float_dtype(res3["baz"]) | ||
assert isna(res3["baz"].drop(["foobar"])).all() | ||
res._set_value("foobar", "baz", 5) | ||
assert is_float_dtype(res["baz"]) | ||
assert isna(res["baz"].drop(["foobar"])).all() | ||
msg = "could not convert string to float: 'sam'" | ||
with pytest.raises(ValueError, match=msg): | ||
res3._set_value("foobar", "baz", "sam") | ||
res._set_value("foobar", "baz", "sam") | ||
|
||
def test_set_value_with_index_dtype_change(self): | ||
df_orig = DataFrame(np.random.randn(3, 3), index=range(3), columns=list("ABC")) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -364,16 +364,14 @@ def test_setitem_dtypes(): | |
|
||
def test_set_value(datetime_series, string_series): | ||
idx = datetime_series.index[10] | ||
res = datetime_series._set_value(idx, 0) | ||
assert res is datetime_series | ||
datetime_series._set_value(idx, 0) | ||
assert datetime_series[idx] == 0 | ||
|
||
# equiv | ||
s = string_series.copy() | ||
res = s._set_value("foobar", 0) | ||
assert res is s | ||
assert res.index[-1] == "foobar" | ||
assert res["foobar"] == 0 | ||
s._set_value("foobar", 0) | ||
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. same here, assert return value is None |
||
assert s.index[-1] == "foobar" | ||
assert s["foobar"] == 0 | ||
|
||
s = string_series.copy() | ||
s.loc["foobar"] = 0 | ||
|
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.
we likely don't have many tests which do this on EAs FYI