ENH: support zoneinfo tzinfos #46425
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,3 +18,4 @@ dependencies: | |
| - numpy | ||
| - python-dateutil | ||
| - pytz | ||
| - backports.zoneinfo | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,7 @@ from datetime import ( | |
| timedelta, | ||
| timezone, | ||
| ) | ||
| from zoneinfo import ZoneInfo | ||
|
|
||
| from cpython.datetime cimport ( | ||
| datetime, | ||
|
|
@@ -41,7 +42,7 @@ cdef int64_t NPY_NAT = get_nat() | |
| cdef tzinfo utc_stdlib = timezone.utc | ||
| cdef tzinfo utc_pytz = UTC | ||
| cdef tzinfo utc_dateutil_str = dateutil_gettz("UTC") # NB: *not* the same as tzutc() | ||
| cdef tzinfo utc_zoneinfo = ZoneInfo("UTC") | ||
|
There was a problem hiding this comment. You probably don't want this to be called unconditionally. Practically speaking, this will always exist, but it's not guaranteed by the There was a problem hiding this comment. OK. Is the failure mode something like "UTC" not being present in /usr/share/zoneinfo/? The user-facing downside of not doing this here is things going through slightly slower code paths. Not the end of the world, but worth avoiding if it doesn't require too much gymnastics. There was a problem hiding this comment. Yes, or if the user is on Windows and doesn't have I imagine it's pretty easy to make the impact of this minimal. One way to do it: cdef tzinfo utc_zoneinfo = None
cdef bool is_utc_zoneinfo(tzinfo tz):
global utc_zoneinfo
if utc_zoneinfo is None:
try:
utc_zoneinfo = ZoneInfo("UTC")
except ZoneInfoNotFoundError:
return False
return tz is utc_zoneinfoPresumably this function will get inlined wherever you call it, and in the "common case" where If you are very concerned with performance I'd probably be trying to lazy-import |
||
|
|
||
| # ---------------------------------------------------------------------- | ||
|
|
||
|
|
@@ -51,9 +52,15 @@ cpdef inline bint is_utc(tzinfo tz): | |
| or tz is utc_stdlib | ||
| or isinstance(tz, _dateutil_tzutc) | ||
| or tz is utc_dateutil_str | ||
| # NB: we are assuming the user does not clear zoneinfo cache | ||
| or tz is utc_zoneinfo | ||
| ) | ||
|
|
||
|
|
||
| cdef inline bint is_zoneinfo(tzinfo tz): | ||
| return isinstance(tz, ZoneInfo) | ||
|
|
||
|
|
||
| cdef inline bint is_tzlocal(tzinfo tz): | ||
| return isinstance(tz, _dateutil_tzlocal) | ||
|
|
||
|
|
@@ -210,6 +217,8 @@ cdef inline bint is_fixed_offset(tzinfo tz): | |
| return 1 | ||
| else: | ||
| return 0 | ||
| elif is_zoneinfo(tz): | ||
| return False | ||
|
There was a problem hiding this comment. Why Also, this isn't quite right. You can tell if a This also works for This whole function can probably be simplified to: if treat_tz_as_dateutil(tz):
if len(tz._trans_idx) == 0 and len(tz._trans_list) == 0:
return 1
else:
return 0
return 0 if tz.utcoffset(None) is None else 1Of course, even that will break when the next version of There was a problem hiding this comment.
No good reason; I'll change it to match.
Thanks. I'll probably make a dedicated branch to both Do This Right and audit our usages of is_fixed_offset, which I don't think we're very consistent about. |
||
| # This also implicitly accepts datetime.timezone objects which are | ||
| # considered fixed | ||
| return 1 | ||
|
|
@@ -264,6 +273,8 @@ cdef object get_dst_info(tzinfo tz): | |
| # e.g. pytz.FixedOffset, matplotlib.dates._UTC, | ||
| # psycopg2.tz.FixedOffsetTimezone | ||
| num = int(get_utcoffset(tz, None).total_seconds()) * 1_000_000_000 | ||
| # If we have e.g. ZoneInfo here, the get_utcoffset call will return None, | ||
| # so the total_seconds() call will raise AttributeError. | ||
| return (np.array([NPY_NAT + 1], dtype=np.int64), | ||
| np.array([num], dtype=np.int64), | ||
| "unknown") | ||
|
|
@@ -291,13 +302,13 @@ cdef object get_dst_info(tzinfo tz): | |
| # deltas | ||
| deltas = np.array([v.offset for v in ( | ||
| tz._ttinfo_before,) + tz._trans_idx], dtype='i8') | ||
| deltas *= 1_000_000_000 | ||
| typ = 'dateutil' | ||
|
|
||
| elif is_fixed_offset(tz): | ||
| trans = np.array([NPY_NAT + 1], dtype=np.int64) | ||
| deltas = np.array([tz._ttinfo_std.offset], | ||
| dtype='i8') * 1_000_000_000 | ||
| typ = 'fixed' | ||
| else: | ||
| # 2018-07-12 this is not reached in the tests, and this case | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an unconditional import — do you not require support for Python < 3.8?
If so, you should presumably have some more complicated logic here and in the
is_zoneinfologic.Something like this works well for "is this an instance of X" without forcing the user to import the module that contains X:
https://github.com/pganssle/pytz-deprecation-shim/blob/bf0174adfce698e280c623d17fe8e82961de0c48/src/pytz_deprecation_shim/helpers.py#L11-L30
https://github.com/pganssle/pytz-deprecation-shim/blob/bf0174adfce698e280c623d17fe8e82961de0c48/src/pytz_deprecation_shim/_common.py#L6-L13
Since obviously some object isn't going to be an instance of
zoneinfo.ZoneInfoif the interpreter has never imported thezoneinfomodule.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think our minimum version is 3.8, but I'm on 3.9 locally and forgot when writing this branch. Will change to try/except.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no leave this
we support >= 3.8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
zoneinfo isnt stdlib until 3.9, so importing unconditionally would require making backports.zoneinfo a hard dependency (and having that in the CI dep file is causing problems AFAICT)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even with
backports.zoneinfoit needs to be conditional since they're in different namespaces, plus I really like the approach I took in thepytzdeprecation shims for checking if a time zone is apytzzone, where my checks never actually importpytzunless it's already been imported (since obviously if it's never been imported then I know the object isn't apytztype). That may be overkill, but it's also not that hard to implement, so ⚖️ 🤷There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll need to take a closer look at the pytz deprecation shim. We're getting rid of a lot of pytz usage in 2.0 (#34916) and these days I keep finding ways we could simplify the code even more if we dropped pytz support altogether.