-
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
[BUG] Make name attr of Index fast slow attrs #16270
[BUG] Make name attr of Index fast slow attrs #16270
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.
Shouldn't it be name
rather than _name
?
/ok to test |
/okay to test |
def name(self): | ||
return self._fsproxy_wrapped._name | ||
|
||
|
||
def Index__setattr__(self, name, value): |
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.
With the usage of _FastSlowAttribute
I don't think we need Index__setattr__
anymore.
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.
I agree, except for MultiIndex.
edit: It might need some more customization because it doesn't just yet without Index__setattr__
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.
Wouldn't defining names
Fastslow attribute for MultiIndex just do the job?
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.
names
attr in Pandas is set with property
like this. I think_set_names
is not being picked up for some reason
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.
pd.MultiIndex([]).names = ... isn't doing anything
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.
No I think we still need Index__setattr__
in Index types. This issue address the problem described here.
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.
No I think we still need
Index__setattr__
in Index types. This issue address the problem described here.
I feel if we do this for one attribute we will have to end up doing the same for every attribute, which is why I think there could be a better approach to solve this problem. Let's pair on this tomorrow ?
/ok to test |
/ok to test |
/merge |
Description
Debugging the spike in failures from #16234
Checklist