#10863 closed defect (fixed)
fix to_datetime() when given timezone naive datetime input
Reported by: | Christian Boos | Owned by: | Christian Boos |
---|---|---|---|
Priority: | normal | Milestone: | 0.12.5 |
Component: | general | Version: | 0.12-stable |
Severity: | normal | Keywords: | datetime pytz timezone |
Cc: | Branch: | ||
Release Notes: | |||
API Changes: |
|
||
Internal Changes: |
Description
While looking at some specifics of the trac.util.datefmt.to_datetime
function, I discovered that a particular usage pattern is problematic with pytz timezones:
>>> from trac.util.datefmt import * >>> from datetime import datetime >>> t = to_datetime(datetime(2012,1,1), timezone("Europe/Paris")) >>> t.utcoffset() datetime.timedelta(0, 540)
(from gmessage:trac-dev:V0HeDyfa_pk/5r33UatF3nEJ)
The utcoffset()
plays a role when converting a time to the number of ms (to_utimestamp()
). Here, it has a value of 9 minutes, which is quite wrong for our purpose.
The workaround is to always specify a timezone aware datetime
object to to_datetime
, but this is not documented, and shouldn't be necessary either (assuming a timezone naive datetime
should actually be the same as a datetime
in utc
).
The following should
-
trac/util/datefmt.py
diff --git a/trac/util/datefmt.py b/trac/util/datefmt.py index b59a276..d88d6b1 100644
a b def to_datetime(t, tzinfo=None): 66 66 elif isinstance(t, datetime): 67 67 if tzinfo is not None: 68 68 if t.tzinfo is None: 69 t = t .replace(tzinfo=tzinfo)69 t = tzinfo.localize(t) 70 70 else: 71 71 t = tzinfo.normalize(t.astimezone(tzinfo)) 72 72 return t
With this change, the t.utcoffset()
for the example above would give 3600 seconds, i.e. the one hour we would expect.
Attachments (1)
Change History (11)
comment:1 by , 12 years ago
Milestone: | → 0.12.5 |
---|
comment:2 by , 12 years ago
To be complete, it seems that datetime.fromtimestamp()
doesn't have the problem that the datetime
constructor has:
>>> datetime.fromtimestamp(1325376000, timezone("Europe/Paris")).utcoffset() datetime.timedelta(0, 3600)
So we don't need to change the other parts of to_datetime()
.
comment:3 by , 12 years ago
Same for datetime.now()
, it's OK as well:
>>> datetime.now(timezone("Europe/Paris")).utcoffset() datetime.timedelta(0, 7200)
(see also r8857)
comment:4 by , 12 years ago
It looks good except datetime between DST boundaries. tzinfo.localize()
generates invalid datetime between DST boundaries. I think we should use tzinfo.normalize()
.
>>> tz = timezone('Europe/Paris') >>> t = datetime(2012, 3, 25, 2, 15) >>> tz.localize(t) datetime.datetime(2012, 3, 25, 2, 15, tzinfo=<DstTzInfo 'Europe/Paris' CET+1:00:00 STD>) >>> tz.localize(t).utcoffset() datetime.timedelta(0, 3600) >>> tz.normalize(tz.localize(t)) datetime.datetime(2012, 3, 25, 3, 15, tzinfo=<DstTzInfo 'Europe/Paris' CEST+2:00:00 DST>) >>> tz.normalize(tz.localize(t)).utcoffset() datetime.timedelta(0, 7200)
-
trac/util/datefmt.py
diff --git a/trac/util/datefmt.py b/trac/util/datefmt.py index b59a276..0fd4aa8 100644
a b def to_datetime(t, tzinfo=None): 66 66 elif isinstance(t, datetime): 67 67 if tzinfo is not None: 68 68 if t.tzinfo is None: 69 t = t.replace(tzinfo=tzinfo) 69 t = tzinfo.localize(t) 70 t = tzinfo.normalize(t) 70 71 else: 71 72 t = tzinfo.normalize(t.astimezone(tzinfo)) 72 73 return t
comment:6 by , 12 years ago
I've tried a more "radical" approach, by making to_datetime(t, tz)
always:
- map the given input time to the given timezone (or
localtz
if no tz is specified) - return a normalized datetime (also going in the direction of #10864)
I think it's a more consistent API and it's easier to use and to remember what it does. All tests still pass.
comment:8 by , 12 years ago
API Changes: | modified (diff) |
---|---|
Resolution: | → fixed |
Status: | new → closed |
Version: | → 0.12-stable |
Applied in r11397. Thanks for the review!
comment:9 by , 12 years ago
Keywords: | pytz timezone added |
---|
comment:10 by , 12 years ago
Owner: | set to |
---|
I think it's OK for 0.12-stable. Feedback welcome!