CLN: trim unreachable indexing code #31768
Conversation
WillAyd
left a comment
WillAyd
left a comment
There was a problem hiding this comment.
This is all just internal right?
| 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.
Shouldn't need to annotate like this; if anything should set the return of get_indexer to np.ndarray
There was a problem hiding this comment.
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
Yes |
jreback
added
the
Indexing
jreback
left a comment
jreback
left a comment
There was a problem hiding this comment.
small assert; i think you can do it in this PR. ping on green.
| raise | ||
|
|
||
| if not isinstance(key, (list, np.ndarray, Series, Index)): | ||
| if not isinstance(key, (list, np.ndarray, ExtensionArray, Series, Index)): |
There was a problem hiding this comment.
we likely don't have many tests which do this on EAs FYI
| 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.
you could assert the return value is None
| 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.
same here, assert return value is None
|
ping |
|
thanks |
3 participants
make _set_value not return anything (the DataFrame docstring in particular is misleading)