REF: delegate attrs for CategoricalIndex, IntervalIndex #30605
Conversation
jreback
added
Categorical
| def _delegate_method(self, name, *args, **kwargs): | ||
| """ method delegation to the ._values """ | ||
| method = getattr(self._values, name) | ||
| if "inplace" in kwargs: | ||
| raise ValueError("cannot use inplace with CategoricalIndex") | ||
| res = method(*args, **kwargs) | ||
| if is_scalar(res): | ||
| if is_scalar(res) or name in self._raw_inherit: |
There was a problem hiding this comment.
can you add a comment here on what is happening
There was a problem hiding this comment.
is_scalar is the heavier check, can you put ‘name in self._raw_inherit’ as the first check.
| @@ -192,6 +214,8 @@ class IntervalIndex(IntervalMixin, Index): | |||
| # Immutable, so we are able to cache computations like isna in '_mask' | |||
| _mask = None | |||
|
|
|||
| _raw_inherit = {"_ndarray_values", "__array__", "overlaps", "contains"} | |||
There was a problem hiding this comment.
same as above, should we also add this attribute as an empty dict in Index?
There was a problem hiding this comment.
id rather not have a dummy attr on Index, but im building up to implementing ExtensionIndex which will have it
| """ method delegation to the ._data """ | ||
| method = getattr(self._data, name) | ||
| res = method(*args, **kwargs) | ||
| if is_scalar(res) or name in self._raw_inherit: |
There was a problem hiding this comment.
comment.
can we not put these methods on Index?
There was a problem hiding this comment.
You mean remove them from IntervalIndex? No idea. @jschendel?
There was a problem hiding this comment.
no the delegation methods that you added could be on Index itself.
There was a problem hiding this comment.
i see. yes, the point of this exercise is to build up to moving as much of it to to either ExtensionIndex or even Index as possible. Doing this on II/CI before refactoring is just to limit the scope.
There was a problem hiding this comment.
its fine, but then will have to clean later
| klass="IntervalIndex", | ||
| examples=textwrap.dedent( | ||
| """\ | ||
| >>> intervals = pd.IntervalIndex.from_tuples([(0, 1), (1, 3), (2, 4)]) |
There was a problem hiding this comment.
It looks like this means that we'll lose any IntervalIndex specific portions of docstrings? And the docstring will just be whatever the IntervalArray docstring is? I'm fine with this since the usage is basically identical between the two so I don't think it will cause any confusion. Will allow for some cleanup of the docstrings on the IntervalArray side but that can be done later.
There was a problem hiding this comment.
It looks like this means that we'll lose any IntervalIndex specific portions of docstrings?
Yes
|
thanks @jbrockmendel comments above, but i think you have already noted them |
Working towards creating an ExtensionIndexMixin to be shared by all our backed-be-EA indexes, which would ideally grow into ExtensionIndex.
Removes CategoricalIndex._codes_for_groupby, which would raise AttributeError if it were ever called.