-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
REF: implement indexes.extension to share delegation #30629
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
Conversation
pandas/core/indexes/extension.py
Outdated
from pandas.util._decorators import cache_readonly | ||
|
||
|
||
def inherit_from_data(name, delegate, cache=False): |
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 you annotate?
|
||
method = property(fget, fset) | ||
|
||
elif not callable(attr): |
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 somewhat confused by this function. Documented as working on methods
but seems to accept attributes as well?
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.
Will update docstring. This clause is for regular class attributes, in particular a few List[str] that we share
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.
lgtm. very minor comments; does this properly preserve doc-strings? i guess no easy way to test this
"_field_ops", | ||
"_datetimelike_ops", | ||
"_datetimelike_methods", | ||
], |
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.
you have the cache=True ones first in datetimelike, can you conform
return method | ||
|
||
|
||
def inherit_names(names: List[str], delegate, cache: bool = False): |
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.
you could type this -> Callable[[Type[T]], Type[T]] i think
k merge on green. |
green |
thanks |
In following steps, I plan to