TYP: Added type hint to Index.repeat #54739
Conversation
pandas/core/indexes/base.py
Outdated
| @@ -1205,7 +1206,7 @@ def _maybe_disallow_fill(self, allow_fill: bool, fill_value, indices) -> bool: | |||
| """ | |||
|
|
|||
| @Appender(_index_shared_docs["repeat"] % _index_doc_kwargs) | |||
| def repeat(self, repeats, axis=None): | |||
| def repeat(self, repeats, axis: AxisInt | None = None): | |||
There was a problem hiding this comment.
I would prefer to change it to axis: None = None as the docs say:
Must be None. Has no effect but is accepted for compatibility with numpy.
There was a problem hiding this comment.
Same applies to the other two changes
There was a problem hiding this comment.
Sure, no issues when I change it to axis: None = None in repeat. However, I got the error below from type validation when I changed it in argmin:
pandas/core/indexes/base.py:7276: error: Argument 1 of "argmin" is incompatible with
supertype "IndexOpsMixin"; supertype defines the argument type as "int | None" [override]Same for argmax. Should I change it to axis: int | None = None for these two then?
There was a problem hiding this comment.
The doc-string in IndexOpsMixin.argmin/max also says that it is only there for compatibility. So I would be tempted to change even IndexOpsMixin.argmin/max to axis: None = None. One could also make the argument that the type should be int | None as both are valid :) Let's make the change for at least repeat to be in sync with the doc-string but the other cases are up to you.
@mroeschke IndexOpsMixin.argmin "uses" axis exactly once - nv.validate_minmax_axis(axis). Testing an unused argument seems not very helpful but removing it probably changes the runtime behavior when people expect to get an error.
There was a problem hiding this comment.
Yeah ideally this should still error the same if a non None axis was passed
There was a problem hiding this comment.
When I change IndexOpsMixin.argmin to axis: None = None, I get the error below from type validation:
pandas/core/series.py:2557: error: Argument 1 to "argmin" of "IndexOpsMixin" has
incompatible type "int"; expected "None" [arg-type]Series.idxmin is using IndexOpsMixin.argmin.
If nv.validate_minmax_axis(axis) function is staying as it is, I could work on the type hints related to this separately.
I realised it is also used in other functions for compatibility and the adopted type hint is axis: AxisInt | None = None (e.g. SparseArray.min, IntervalArray.min, etc.) .
Currently, there is no type hint for axis in the following functions:
Index.argmax
Index.argmin
Index.max
Index.min
RangeIndex.max
RangeIndex.min
mecopur
changed the title
mecopur
changed the title
mecopur
changed the title
|
Thanks @mecopur |
Added type hint to Index.repeat
doc/source/whatsnew/vX.X.X.rstfile if fixing a bug or adding a new feature.@noatamir @phofl