REF: simplify PeriodIndex.get_loc #31598
Conversation
jbrockmendel
added
the
Indexing
| try: | ||
| return self._engine.get_loc(ordinal) | ||
| return Index.get_loc(self, key, method, tolerance) |
There was a problem hiding this comment.
I wonder if this changes perf? the reason is that we have already created an _int64index, but now we are creating something new?
There was a problem hiding this comment.
but now we are creating something new?
I don't think we're creating a new object here
There was a problem hiding this comment.
the engine itself is (maybe) being created again
There was a problem hiding this comment.
i guess on the first all, but on the flip side the _int64index may not have been created yert
| @@ -153,7 +153,6 @@ def test_setitem_ndarray_3d(self, index, obj, idxr, idxr_id): | |||
| "floating", | |||
| "string", | |||
| "datetime64", | |||
| "period", | |||
There was a problem hiding this comment.
on the Series.__setitem__ case it now (correctly, i think) raises, as it is trying to instantiate an index with 3D data
There was a problem hiding this comment.
this test is a mess, is hiding the fact that all of the cases in this block should raise but currently dont (xref #31813)
|
can you rebase |
|
rebased+green |
This makes PeriodEngine follow patterns in DatetimeEngine, so soon we can have it subclass DatetimeEngine instead of Int64Engine, which will lead to more simplifications.
This also puts us within striking distance of sharing get_loc code