-
Notifications
You must be signed in to change notification settings - Fork 915
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
Avoid redefining Frame._get_columns_by_label in subclasses #15912
Avoid redefining Frame._get_columns_by_label in subclasses #15912
Conversation
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 @mroeschke! One question but overall LGTM
if by is None: | ||
to_sort = self._columns | ||
else: | ||
to_sort = self._get_columns_by_label(list(by))._columns |
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.
Outside of dropping of downcast
, is there any reason for the restructure here? Are there cases where we would expect the unpacking here not to work?
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.
This was mostly a micro-optimization to avoid the unnecessary unpacking of the if by is None
case
/merge |
Description
Frame._get_columns_by_label
was redefined inSeries
andDataFrame
to handle some special edge cases inDataFrame.__getitem__
and emptySeries
By making
_from_data_like_self
more consistent in preserving external properties and moving special casing, we can only defineFrame._get_columns_by_label
onceChecklist