-
-
Notifications
You must be signed in to change notification settings - Fork 18k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
TYP: type NDFrame.(_get_axis|_get_axis_name|_get_axis_number) #33610
TYP: type NDFrame.(_get_axis|_get_axis_name|_get_axis_number) #33610
Conversation
except AttributeError as err: | ||
raise TypeError("Index must be DatetimeIndex") from err | ||
if not isinstance(index, DatetimeIndex): | ||
raise TypeError("Index must be DatetimeIndex") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
name = self._get_axis_name(axis) | ||
return getattr(self, name) | ||
|
||
@classmethod | ||
def _get_block_manager_axis(cls, axis): | ||
def _get_block_manager_axis(cls, axis) -> int: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can "axis" be typed as Axis?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, but in some cases axis
is returned, so that that would require some refactoring inside the function, which I'd like to do seperately. For example I'm pretty sure _AXIS_ALIASES
can be removed from the code base.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
makes sense, thanks
LGTM; @simonjayhawkins might have an opinion about the "type: ignore" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @topper-123 generally lgtm. can you add comments when adding #type: ignore to help reviewers and help track.
pandas/core/generic.py
Outdated
@@ -3490,6 +3490,7 @@ class animal locomotion | |||
axis = self._get_axis_number(axis) | |||
labels = self._get_axis(axis) | |||
if level is not None: | |||
assert isinstance(labels, MultiIndex), type(labels) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could this raise a user-facing AssertionError?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, Changed it to raise a TypeError, which is also better than an AttributeError in master.
@@ -590,7 +590,7 @@ def swapaxes(self: FrameOrSeries, axis1, axis2, copy=True) -> FrameOrSeries: | |||
if copy: | |||
new_values = new_values.copy() | |||
|
|||
return self._constructor(new_values, *new_axes).__finalize__( | |||
return self._constructor(new_values, *new_axes).__finalize__( # type: ignore |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm, can you add the mypy error as a comment.
IIUC there were issues with deprecating this method, xref #26654.
For series, it's a no-op. maybe now that Panel is deprecated, we could have logic in DataFrame and Series instead of NDFrame and simplify.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I remember that. The problem was that np.swapaxes(df, 0, 1)
, np.swapaxes(series, 0, 0)
works in master and are tested for. It's not unreasonable that it works, but users should really use transpose
/T
instead.
Maybe add swapaxes
to _deprecations
to get it out of the tab-list?
4dff9c3
to
fadfb0a
Compare
Updated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice updates
Gives return types to
NDFrame._get_axis
etc.