ENH: implement ExtensionIndex.insert #32476
Conversation
| @@ -242,6 +243,10 @@ def repeat(self, repeats, axis=None): | |||
| result = self._data.repeat(repeats, axis=axis) | |||
| return self._shallow_copy(result) | |||
|
|
|||
| def insert(self, loc: int, item): | |||
| # ExtensionIndex subclasses must override Index.insert | |||
There was a problem hiding this comment.
Any reason not to make an abstractmethod?
There was a problem hiding this comment.
So this I believe is ideal but any concerns on backward compat? Otherwise lgtm
There was a problem hiding this comment.
any concerns on backward compat?
Nothing comes to mind
There was a problem hiding this comment.
Gothca. This is non-public right?
There was a problem hiding this comment.
ExtensionIndex needs to inherit from abc.ABC in order for the abstractmethod decorator to actually work, right?
In [1]: import abc
In [2]: class Foo:
...: @abc.abstractmethod
...: def some_meth(self):
...: pass
...:
In [3]: class Bar(Foo):
...: pass
...:
In [4]: bar = Bar()
In [5]: class Foo2(abc.ABC):
...: @abc.abstractmethod
...: def some_meth(self):
...: pass
...:
In [6]: class Bar2(Foo2):
...: pass
...:
In [7]: bar2 = Bar2()
---------------------------------------------------------------------------
TypeError: Can't instantiate abstract class Bar2 with abstract methods some_meth
There was a problem hiding this comment.
lgtm ex #32476 (comment)
also, since the type of Index returned depends on the inserted item may be worth adding return types so we get mypy checking consistency.
|
The abc.abstractmethod didn't appear to function as intended even when I subclassed abc.ABC, so reverted to use AbstractMethodError. |
|
comments addressed, sort of. |
|
seems fine |
Sort of.