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

ENH: Add timetz attribute to DatetimeIndex and Timestamp #21358

Closed
mroeschke opened this issue Jun 7, 2018 · 11 comments

Comments

Projects
None yet
4 participants
@mroeschke
Copy link
Member

commented Jun 7, 2018

Currently DatetimeIndex.time returns a numpy array of local datetime.times. In #18461, the resulting datetime.times gained timezone information (but was an undocumented changed) and was subsequently reverted in #21281. Having .time return timezone information would still be a nice enhancement.

If we allow DatetimeIndex.time to return timezone information, timeseries.rst will need to be changed since it currently indicates that .time does not return timezone information.

@jquinon

This comment has been minimized.

Copy link
Contributor

commented Jul 6, 2018

Can I work on this? I'm new to working on pandas, but this seems like a fairly simple issue to get started with.

@mroeschke

This comment has been minimized.

Copy link
Member Author

commented Jul 6, 2018

Go for it @jquinon! The main job would be to undo the revision in #21281 make sure this change is documented.

@mroeschke

This comment has been minimized.

Copy link
Member Author

commented Jul 20, 2018

So now DatetimeIndex.time matches datetime.time() (minus the property vs method difference) which is an ideal consistency.

However, there is a datetime.timetz() method that returns time with timezone information. We could match the api and have a DatetimeIndex.timetz method, but I am not the biggest fan of the name.

I would prefer instead having DatetimeIndex.time(tz=False) that would match the method-like time api of the standard library, but use a keyword arg to return tz information.

Thoughts?

@jreback

This comment has been minimized.

Copy link
Contributor

commented Jul 20, 2018

yeah that would be ok except it’s current a property - not sure we can change this

@mroeschke

This comment has been minimized.

Copy link
Member Author

commented Jul 20, 2018

It would be an API change; otherwise, I don't recall an internal reason why we can't break it.

@jorisvandenbossche

This comment has been minimized.

Copy link
Member

commented Jul 20, 2018

I am not sure this (adding a tz keyword) is worth the API change. Changing from a attribute to method is a quite big change, as every use of the current attribute would be impacted.

@mroeschke

This comment has been minimized.

Copy link
Member Author

commented Jul 20, 2018

Fair enough. Should we then mimic the standard library and add a .timetz property?

@jorisvandenbossche

This comment has been minimized.

Copy link
Member

commented Jul 23, 2018

Should we then mimic the standard library and add a .timetz property?

I personally don't really care about adding it, but also won't object if others want it.

@mroeschke mroeschke closed this Jul 23, 2018

@mroeschke mroeschke reopened this Jul 23, 2018

@mroeschke

This comment has been minimized.

Copy link
Member Author

commented Jul 23, 2018

@jquinon if you're interested to add a timetz property to DatetimeIndex and Timestamp

@mroeschke mroeschke changed the title ENH: Allow DatetimeIndex.time to return timezone information ENH: Add timetz attribute to DatetimeIndex and Timestamp Jul 23, 2018

@jquinon

This comment has been minimized.

Copy link
Contributor

commented Jul 23, 2018

If everyone is cool with it then sure. @jreback

@jreback

This comment has been minimized.

Copy link
Contributor

commented Jul 23, 2018

yep this sounds ok

@jquinon jquinon referenced this issue Jul 30, 2018

Merged

Add timetz attribute to DatetimeIndex #22132

4 of 4 tasks complete

@jreback jreback added this to the 0.24.0 milestone Aug 9, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.