-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
API/CLN: add in common operations to Series/Index, refactored as a OpsMixin #6380
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
This looks cool. Slightly OT would it be useful to have a method to split dates into their parts? Something like split on datetime index? Would be useful for group by days months etc. although maybe there's a way to do that now |
Or date_split since split is a bit too general sounding |
you can just do I have to put a list of follow up stuff.... |
With this PR (note that DataFrame support missing for this, and I am not sure that we should add it
|
@jorisvandenbossche I have to push an update because the doc strings are not set... so I can get
to work properly but ipython help is odd
any idea how to do this? |
@jreback Do you mean that you get a verbose output fget/fset for properties with the ipython help |
yes exactly hmm ok so this is a known issue then I put these in the API as well |
@cpcloud @jtratner @jorisvandenbossche any more comments on this? |
# facilitate the properties on the wrapped ops | ||
def _field_accessor(name, docstring=None): | ||
op_accessor = '_{0}'.format(name) | ||
def f(self): |
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.
Did you mean to use @wraps
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.
I think I originally did, but sort of did it 'manually' by assigning name/doc string....bad?
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.
Looking at the source for wraps
it basically does that ... only diff is that it updates the __module__
attribute and copies the __dict__
attribute. Not "bad" per se.
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 is fine ... I was just asking.
…sMixin (GH4551, GH4056, GH5519)
@cpcloud I took wraps out...not necessary as I am creating the function wrappers here from an accesssor, rather than wrapping another function |
API/CLN: add in common operations to Series/Index, refactored as a OpsMixin
closes #4551
closes #4056
closes #5519
allow a Series to utilize index methods for its index type, e.g.
Series.year
is now definedfor a Series with a
DatetimeIndex
or aPeriodIndex
; trying this on an Index type willnow raise a
TypeError
. The following properties are affected:date,time,year,month,day,hour,minute,second,weekofyear
week,dayofweek,dayofyear,quarter,microsecond,nanosecond,qyear
and methods:
min(),max()
,