Skip to content
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

API/INT: why is to_pydatetime handled differently in Series.dt accessor? #20306

Closed
jorisvandenbossche opened this issue Mar 12, 2018 · 3 comments · Fixed by #52459
Closed

API/INT: why is to_pydatetime handled differently in Series.dt accessor? #20306

jorisvandenbossche opened this issue Mar 12, 2018 · 3 comments · Fixed by #52459
Labels
Datetime Datetime data dtype Enhancement

Comments

@jorisvandenbossche
Copy link
Member

jorisvandenbossche commented Mar 12, 2018

While reviewing: #20198, I notices this was defined in the DatetimeProperties itself:

class DatetimeProperties(Properties):
"""
Accessor object for datetimelike properties of the Series values.
Examples
--------
>>> s.dt.hour
>>> s.dt.second
>>> s.dt.quarter
Returns a Series indexed like the original Series.
Raises TypeError if the Series does not contain datetimelike values.
"""
def to_pydatetime(self):
return self._get_values().to_pydatetime()
@property
def freq(self):
return self._get_values().inferred_freq
DatetimeProperties._add_delegate_accessors(
delegate=DatetimeIndex,
accessors=DatetimeIndex._datetimelike_ops,
typ='property')
DatetimeProperties._add_delegate_accessors(
delegate=DatetimeIndex,
accessors=DatetimeIndex._datetimelike_methods,
typ='method')

Is there any reason that we have it like that, while all others are just wrapped DatetimeIndex attributes/methods?

Because that means this returns (a bit strangly) an ndarray instead of a Series of the datetime.datetime values (making it inconsistent with other methods)

cc @jbrockmendel @jreback

@jorisvandenbossche
Copy link
Member Author

Ah, now I opened this issue, I thought of a probable reason: because otherwise the datetime.datetime values would quickly get inferred / coerced back to datetime64 ?

@jorisvandenbossche jorisvandenbossche added the Datetime Datetime data dtype label Mar 12, 2018
@jreback
Copy link
Contributor

jreback commented Mar 12, 2018

yes that is the reason
though we could return an object dtype Index

@jbrockmendel
Copy link
Member

Could change this for 2.0. Hard to deprecate since there isn't a good "do X instead."

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Datetime Datetime data dtype Enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants