Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # 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."

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

Successfully merging a pull request may close this issue.

4 participants