REF: de-duplicate NDFrame.take, remove Manager.take keyword #51482
Conversation
| @@ -3916,8 +3903,14 @@ def _take( | |||
| and is_range_indexer(indices, len(self)) | |||
| ): | |||
| return self.copy(deep=None) | |||
| elif self.ndim == 1: | |||
| # TODO: be consistent here for DataFrame vs Series | |||
There was a problem hiding this comment.
@phofl following this ill do do the deprecation we discussed
|
thx @jbrockmendel |
|
@jbrockmendel Looks like this could have caused a performance regression. https://asv-runner.github.io/asv-collection/pandas/#indexing.NumericSeriesIndexing.time_iloc_array?p-index_structure='unique_monotonic_inc'&p-dtype=%3Cclass%20'numpy.int64'%3E Could not reproduce locally though, do you have an idea? |
|
It's plausible. This PR makes us go through Manager.take whereas the old version operated directly on ._values/.index. In retrospect its not that surprising, though I'd have hoped for the effect to be smaller. Restoring Series.take (the rest of this should be OK) should fix it. If we go for something like ExtensionManager mentioned in #51471 then we'd need to have basically everything go through a Manager method, in which case we'd need to optimize Manager.take somehow. |
|
Yep adding Series.take back would get performance back, but as you said not really a long term solution unfortunately |
doc/source/whatsnew/vX.X.X.rstfile if fixing a bug or adding a new feature.