-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
API: add Series.dt delegator for datetimelike methods (GH7207) #7953
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
def __dir__(self): | ||
return sorted(list(set(self.ops))) | ||
|
||
def _maybe_to_datetimelike_index(data, copy=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.
Is that still a index, now it returns a Series
? If not the name should probably be changed
Looks good! Thanks for this. The only thing I was thinking to have it named Only should need some dos (whatsnew, addition to the API (http://pandas.pydata.org/pandas-docs/stable/api.html#time-series-related), and somewhere else in the docs) |
yeh could change the name |
@jorisvandenbossche ok, this nicely builds the API (similar to StringMethods). |
.dt accessor | ||
~~~~~~~~~~~~ | ||
|
||
``Series`` has an accessor to succintly return datetime like properties for the *values* of the Series, if its a datetime/period like Series. (:issue:`7207`) |
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.
succinctly? I don't think there should be a GH link in the basics section?
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.
yep
when accessing a doc-string for a property
|
raise TypeError("cannot convert an object of type {0} to a datetimelike index".format(type(data))) | ||
|
||
# facilitate the properties on the wrapped ops | ||
def _add_accessors(cls): |
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.
@JanSchulz see here
if you have a specific method, then just define it directly (after all this is just a convience to avoid writing each method manuall). which are you referring? |
I've added the possibility for a setter in jankatins@1b52a71 |
gr8; I'll incorporate that and repush |
Re "different methods": I just noticed that when I wrote my implementation of "CategoricalProperties" (in the end it was not needed). |
@JanSchulz updated to incorporate the get/set |
@jreback for the property docs, I think that this is an IPython issue (see ipython/ipython#1555, and fixed by @immerrr in ipython/ipython#4827 which will be in IPython 3.0 I think). |
.dt accessor | ||
~~~~~~~~~~~~ | ||
|
||
``Series`` has an accessor to succinctly return datetime like properties for the *values* of the Series, if its a datetime/period like Series. (:issue:`7207`) |
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.
link to issue is not needed here I think (it is in the whatsnew)
Nice docs! Added some but very minor comments. |
def _delegate_property_get(self, name): | ||
raise NotImplementedError("PandasDelegate is an abstract base class: _delegate_property_get """) | ||
|
||
def _delegate_property_set(self, name): |
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.
potentially this should have new_values arg, so that it raises correctly (atm it's a TypeError, wrong number of args).
Though I don't quite follow how setters will work... would we be allowing things like s.dt.year[0] = 2000
and weird things like this (atm that'll fail silently but perhaps it's possible to make it raise :s).
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.
it's not applicable for indexs (though for series it could work) - it's defined as a base fore Categoical namespace
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, IMO ABC error is going to be a bit confusing when people try and do this (on this branch):
In [11]: s = pd.Series(pd.to_datetime(['2012', '2013']))
In [12]: s.dt.year = [1000, 2000] # if add the new_values this'll be a NotImplemented ABC error.
TypeError: _delegate_property_set() takes exactly 2 arguments (3 given)
In [13]: dt = pd.to_datetime(['2012', '2013'] )
In [14]: dt.year = [1000, 2000] # this error makes sense
AttributeError: can't set attribute
Note: Assigning to the copy temporary dt.year[0] = 2000
always (has) silently failed.
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.
good point
these should always fail
I don't think we should allow modifications here
too complicated / messy and not necessary
(maybe I. the future)
I'll fix the error though
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.
Great! I agree, would be messy, difficult to get right* and slow too.
* maybe even not possible e.g. leap years days and stuff.
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.
In [1]: s = Series(date_range('20130101',periods=3))
In [2]: s.dt.hour = 5
ValueError: cannot set a datetimelike property
In [3]: s.dt.hour[0] = 5
/usr/local/bin/ipython:1: SettingWithCopyWarning:
A value is trying to be set on an immutable datetimelike object
A |
ok in the last what's a 'use this or that ' mean? |
Reading this as a "phd student who does some data munging" would leave me confused: So something like this: |
What about:
This is a great PR, looking forward to this! |
@hayd Much better! |
@JanSchulz @hayd updated to your text for the error messages cc @rosnfeld any other comments? |
.dt accessor | ||
~~~~~~~~~~~~ | ||
|
||
``Series`` has an accessor to succinctly return datetime like properties for the *values* of the Series, if its a datetime/period like Series. |
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.
its -> it's
Looking good to me (just two typo/grammar things), and this is a nice enhancement! |
I pulled the changes and played around with it, looks very good to me. The only minor improvement I'd suggest is a tiny bit more documentation on the actual DateTimeProperties/PeriodProperties code, i.e. compare: In [5]: s.str?
Type: StringMethods
String Form:<pandas.core.strings.StringMethods object at 0x3c23410>
File: /home/andrew/git/pandas-rosnfeld/pandas/core/strings.py
Docstring:
Vectorized string functions for Series. NAs stay NA unless handled
otherwise by a particular method. Patterned after Python's string methods,
with some inspiration from R's stringr package.
Examples
--------
>>> s.str.split('_')
>>> s.str.replace('_', '')
In [6]: s.dt?
Type: DatetimeProperties
String Form:<pandas.tseries.common.DatetimeProperties object at 0x3c06150>
File: /home/andrew/git/pandas-rosnfeld/pandas/tseries/common.py
Docstring: Manages a DatetimeIndex Delegate |
@rosnfeld makes sense, I will emulate |
|
CLN: make _add_delegate_accessors generic
Nice - thanks! |
API: add Series.dt delegator for datetimelike methods (GH7207)
bombs away. |
closes #7207
Tab completion is specific to the type of wrapped delegate (DatetimeIndex or PeriodIndex)