ENH: Change DatetimeIndex.time to also return timezone info

[jquinon]

• closes ENH: Add timetz attribute to DatetimeIndex and Timestamp #21358
• tests added / passed
• passes git diff upstream/master -u -- "*.py" | flake8 --diff
• whatsnew entry

Mainly just reverted changes from GH#21281 and updated docs. Also had to change one test added in GH#21281.

[pep8speaks]

Hello @jquinon! Thanks for updating the PR.

Cheers ! There are no PEP8 issues in this Pull Request. 🍻

Comment last updated on July 11, 2018 at 20:57 Hours UTC

[jschendel]

Missing a closing backtick for the issue number. Can you also add an :attr: ref for DatetimeIndex.time?

[jquinon]

Does that look good?

[codecov]

Codecov Report

Merging #21865 into master will decrease coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #21865      +/-   ##
==========================================
- Coverage   91.92%   91.92%   -0.01%     
==========================================
  Files         160      160              
  Lines       49906    49903       -3     
==========================================
- Hits        45875    45872       -3     
  Misses       4031     4031

Flag	Coverage Δ
#multiple	90.3% <100%> (-0.01%)	⬇️
#single	42.11% <0%> (ø)	⬆️

Impacted Files	Coverage Δ
pandas/core/arrays/datetimes.py	95.79% <100%> (-0.06%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5d0daa0...b7e29cf. Read the comment docs.

[mroeschke]

Could you expand this docstring to also mention that the datetime.time objects are in local time with associated timezone information.

[jquinon]

Does that look good?

[mroeschke]

Small nit: could you replace , and contains with with. Otherwise LGTM

[jquinon]

No problem

[jreback]

does the standard library preserve for datetime.datetime.time() ?

[mroeschke]

.time() does not but .timetz() does:

In [12]: pytz.UTC.localize(datetime(2000, 1, 1)).time()
Out[12]: datetime.time(0, 0)

In [13]: pytz.UTC.localize(datetime(2000, 1, 1)).timetz()
Out[13]: datetime.time(0, 0, tzinfo=<UTC>)

I guess if we want to follow the standard library naming it should be naive, but I think its useful knowing in which timezone the local time resides.

An alternative is to depreciate the .time property for a time(tz=False) method that mirrors the standard library, has an option to return tz information and is maybe less API breaking.

Thoughts @jreback?

[jreback]

hmm its annoying that the std library does this with 2 methods, but I guess can't easily 'remove' a tz from a time (e.g. no .tz_localize(None).

[mroeschke]

Yeah that's why I am more inclined to chose the method route to mimic the standard library but at least have a keyword argument (and not a second method) to include the tz in the output or not.

[jquinon]

I can modify or cancel the PR as you guys see fit, just let me know when a decision is made!

[jorisvandenbossche]

I would personally follow the standard library and leave it as is as a naive time.
You can always get the timezone separately if you want.

[jorisvandenbossche]

@jquinon thanks for working on it anyhow!