ENH: Support datetime.timezone objects by mroeschke · Pull Request #25065 · pandas-dev/pandas (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation20 Commits31 Checks0 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

tests added / passed
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

Introduced in Python 3.2. First pass is to evaluate what tests fail when included in the tz_aware_fixture

Matt Roeschke added 4 commits

January 29, 2019 23:54

Codecov Report

Merging #25065 into master will not change coverage.
The diff coverage is n/a.

@@ Coverage Diff @@ ## master #25065 +/- ##

Coverage 92.37% 92.37%

Files 166 166
Lines 52403 52403

Hits 48405 48405
Misses 3998 3998

Flag	Coverage Δ
#multiple	90.79% <ø> (ø)	⬆️
#single	42.87% <ø> (ø)	⬆️

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 ea013a2...5076045. Read the comment docs.

Codecov Report

Merging #25065 into master will not change coverage.
The diff coverage is n/a.

@@ Coverage Diff @@ ## master #25065 +/- ##

Coverage 91.25% 91.25%

Files 172 172
Lines 52977 52977

Hits 48342 48342
Misses 4635 4635

Flag	Coverage Δ
#multiple	89.82% <ø> (ø)	⬆️
#single	41.74% <ø> (ø)	⬆️

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 707c720...1c6165b. Read the comment docs.

@@ -27,7 +28,7 @@ cdef int64_t NPY_NAT = get_nat()
# ----------------------------------------------------------------------

cpdef inline bint is_utc(object tz):
return tz is UTC or isinstance(tz, _dateutil_tzutc)
return tz is UTC or isinstance(tz, _dateutil_tzutc) or tz is timezone.utc

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider “cdef tzinfo utc_stdlib = timezone.utc” at module level. Much faster lookup for frequently-done check.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wasn't able to cdef with tzinfo (used object instead). Would it tzinfo have to be cimported?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it tzinfo have to be cimported?

Yah, that shouldn't be a problem though. We do it elsewhere.

Matt Roeschke added 4 commits

February 2, 2019 20:37

Python 2 builds are failing as expected.

By virtue of the tz_fixtures, datetime.timezones are now being tested with

DatetimeIndex & Timestamp & date_range & Series constructor
DatetimeIndex & Timestamp methods
timezone conversion
indexing
comparison and arithmetic ops with arrays & scalars
append/concat & other reshaping methods
NaT ops

mroeschke changed the title~~[WIP] ENH: Support datetime.timezone objects~~ ENH: Support datetime.timezone objects

Feb 3, 2019

@@ -20,7 +20,7 @@ Other Enhancements
^^^^^^^^^^^^^^^^^^

- :meth:`Timestamp.replace` now supports the ``fold`` argument to disambiguate DST transition times (:issue:`25017`)
-
- ``datetime.timezone`` objects are now supported (:issue:`25065`)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you add a :meth: with a ref to the python docs

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add where they are supported

Matt Roeschke added 11 commits

February 4, 2019 13:37

Matt Roeschke added 2 commits

February 17, 2019 15:04

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, minor nitpicks if you have a chance.

@@ -2149,12 +2149,9 @@ Time Zone Handling
------------------

pandas provides rich support for working with timestamps in different time
zones using the ``pytz`` and ``dateutil`` libraries.
zones using the ``pytz`` and ``dateutil`` libraries or ``datetime.timezone``

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that :class:datetime.timezone will work.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like there are a few more mentions of what's supported

Reword L272 to include datetime.timezone
Add a daetetime.timezone example in the example ~L2181

@@ -25,7 +25,7 @@ Other Enhancements
- ``Series.str`` has gained :meth:`Series.str.casefold` method to removes all case distinctions present in a string (:issue:`25405`)
- :meth:`DataFrame.set_index` now works for instances of ``abc.Iterator``, provided their output is of the same length as the calling frame (:issue:`22484`, :issue:`24984`)
- :meth:`DatetimeIndex.union` now supports the ``sort`` argument. The behaviour of the sort parameter matches that of :meth:`Index.union` (:issue:`24994`)
-
- :meth:`datetime.timezone` objects are now supported as arguments to timezone methods and constructors (:issue:`25065`)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

meth -> class (not sure if it matters).


# ----------------------------------------------------------------------

cpdef inline bint is_utc(object tz):
return tz is UTC or isinstance(tz, _dateutil_tzutc)
return tz is UTC or isinstance(tz, _dateutil_tzutc) or tz is utc_stdlib

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potential micro optimization: I suspect that tz is utc_stdlib is relatively cheap compared to isinstance(tz, _dateutil_tzutc). Maybe swap the order of those two for a better average case? Not sure if this matters.

Matt Roeschke added 2 commits

March 4, 2019 20:00

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I'd say merge later today if there are no objections.


# ----------------------------------------------------------------------

cpdef inline bint is_utc(object tz):
return tz is UTC or isinstance(tz, _dateutil_tzutc)
return tz is UTC or or tz is utc_stdlib or isinstance(tz, _dateutil_tzutc)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoops, an extra or here.

@TomAugspurger This branch will always fail the PY2 builds though (feature introduced in 3.3). I was thinking we wait until those builds are removed (hopefully soonish/after 0.24.2)