BUG: loc raising KeyError for string slices in list-like indexer and DatetimeIndex #38153
Conversation
…180_no_up_yet � Conflicts: � doc/source/whatsnew/v1.2.0.rst
|
There are a couple of issues here, first of which is that it isn't obvious that we actually should support this. It does improve internal consistency, but adds a lot of complexity. Even if we do decide to move forward with this, id want to start with making get_indexer behave like a vectorized get_loc. (ideally id like to get rid of of the |
+1 here. yeah partial date indexing has only very limited support now, we don't wish to expand this, its mainly useful for interactive use. unless other objections going to close the original issue. thanks @phofl for the effort here. |
|
No worries, I was not sure either if this would be desirable. But wanted to share a possible solution before discussing this. |
jreback
added
the
Needs Discussion
|
did we settle on closing this? |
|
Think so? |
| try: | ||
| for k in key: | ||
| # Convert slices to list of integers | ||
| indexer = positions[self.get_loc(k)] |
There was a problem hiding this comment.
can you use .get_indexer() here? (and still then iterate over the results)
There was a problem hiding this comment.
We discussed not supporting this. Has this changed?
There was a problem hiding this comment.
yah i think we want to close this
| if not isinstance(key, list): | ||
| # There are no slices, so we can dispatch back | ||
| return super()._convert_listlike_indexer(key) | ||
|
|
There was a problem hiding this comment.
actually you can short-circust if we don't have any slices right?
| expected = Series( | ||
| 1, | ||
| index=[ | ||
| Timestamp("2001-01-29"), | ||
| Timestamp("2001-01-30"), | ||
| Timestamp("2001-01-31"), | ||
| Timestamp("2001-01-30"), | ||
| ], | ||
| ) |
There was a problem hiding this comment.
nitpick, but this might look a bit better if you define index = on its own line and then here have expected = Series(1, index=index)
|
closing, though we should have a test and good error message if someone attempst this. @phofl want to repurpose this? |
|
Yep will do |
|
Currently returns which is obviously wrong. I would change this to raise if this is attempted. |
I'm not sure how avoidable this is given that |
|
Yep, you are probably right. Have not found a way to avoid this. Closing this pr. |
black pandasgit diff upstream/master -u -- "*.py" | flake8 --diffI looked into this and added an implementation for list-like indexers containing slices and DatetimeIndexes. If we want to support this we probably have to do something like this.