Opened 16 years ago

Closed 16 years ago

Last modified 12 years ago

#7201 closed (fixed)

timeuntil filter misbehaves if my datetime has a timezone

Reported by: terry@… Owned by: Jeremy Carbaugh
Component: Template system Version: dev
Severity: Keywords: timeuntil, timesince
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Looking in the code, I see that django.utils.timesince carefully tries to compute "now" using an appropriate timezone if you call timesince() with now=Null. However, if you call the timeuntil() routine in that module, or if you use django.template.defaultfilters.timeuntil(), then it computes now using no timezone at all. If the datetime you pass into timeuntil has a timezone set, then the result you get is several hours off.

Attachments (4)

7201.diff (8.0 KB ) - added by Jeremy Carbaugh 16 years ago.
Fixes timezone errors in timeuntil and timesince. Includes tests and documentation updates.
7201.2.diff (7.3 KB ) - added by Jeremy Carbaugh 16 years ago.
Modification of original patch to not assume timezone when none exists.
7201.3.diff (7.8 KB ) - added by Jeremy Carbaugh 16 years ago.
This patch is the same as 7201.2.diff with an added change to AUTHORS per Malcolm's request.
7201.4.diff (7.9 KB ) - added by Jeremy Carbaugh 16 years ago.
Updated to work with changes introduced in [8535]

Download all attachments as: .zip

Change History (15)

comment:1 by Sung-jin Hong, 16 years ago

Component: UncategorizedTemplate system
Keywords: timeuntil timesince added
milestone: 1.0
Triage Stage: UnreviewedAccepted

Seems to be a clear bug.

comment:2 by Leah Culver, 16 years ago

Owner: changed from nobody to Leah Culver
Status: newassigned

comment:3 by Leah Culver, 16 years ago

Owner: Leah Culver removed
Status: assignednew

comment:4 by Jeremy Carbaugh, 16 years ago

Owner: set to Jeremy Carbaugh
Status: newassigned

comment:5 by Jeremy Carbaugh, 16 years ago

Has patch: set

Prior to this patch, the timesince(d, now=None) method would overwrite the timezone of the now argument if the d argument had tzinfo. This would cause the time difference to be calculated incorrectly. Additionally, if d did not have tzinfo, the tzinfo of now would be reset to None. In this case, the patched version will give the d argument the tzinfo of now to keep the behavior the same for both arguments.

This patch raises the issue of whether the timesince method should assume a timezone if one argument has tzinfo and the other does not. Default Python behavior is to raise a TypeError when operating on offset-naive and offset-aware datetimes. I am opening another ticket to address the issue of how this should be handled. Allowing an error to be raised when comparing timezone and non-timezone arguments will be a backwards incompatible change.

comment:6 by Jeremy Carbaugh, 16 years ago

Ticket #8080 has been created to address the behavior of timesince and timeuntil.

by Jeremy Carbaugh, 16 years ago

Attachment: 7201.diff added

Fixes timezone errors in timeuntil and timesince. Includes tests and documentation updates.

by Jeremy Carbaugh, 16 years ago

Attachment: 7201.2.diff added

Modification of original patch to not assume timezone when none exists.

comment:7 by Jeremy Carbaugh, 16 years ago

After thinking about the patch over the weekend and trying a few extra tests, I decided that the original patch did not adequately address the issue. The real bug in the code was not that a timezone was being overwritten, but that a timezone was assumed at all. Even with the bug fixed with the original patch, unexpected output still existed when a timezone was incorrectly assumed.

The current patch fixes the initial bug and any other instance where datetime arguments contain tzinfo. If two arguments are passed, one with tzinfo and one without, the filter will return an empty string as other filters do on TypeError or ValueError.

This patch did not break any previous tests, but may be backwards incompatible in cases where offset-naive and offset-aware datetimes are used as arguments.

I've closed #8080 that I opened earlier as this patch resolves that issue.

comment:8 by Malcolm Tredinnick, 16 years ago

I think you're logic is pretty sound in the revised patch. I think we can wear the backwards incompatibility, since I'd argue that if you're relying on the current behaviour, your results are already wrong.

Can you include a patch for the AUTHORS file as part of this? This isn't a trivial amount of effort to have worked it all out. We should immortalise your efforts.

by Jeremy Carbaugh, 16 years ago

Attachment: 7201.3.diff added

This patch is the same as 7201.2.diff with an added change to AUTHORS per Malcolm's request.

comment:9 by Jeremy Carbaugh, 16 years ago

Thanks for reviewing the patch, Malcolm.

by Jeremy Carbaugh, 16 years ago

Attachment: 7201.4.diff added

Updated to work with changes introduced in [8535]

comment:10 by Malcolm Tredinnick, 16 years ago

Resolution: fixed
Status: assignedclosed

(In [8579]) Fixed #7201 -- Fixed the timeuntil filter to work correctly with timezone-aware
times. Patch from Jeremy Carbaugh.

This is backwards incompatible in the sense that previously, if you tried to
compare timezone-aware and timezone-naive values, you got an incorrect result.
Now you get an empty string. So your previously incorrect code returns a
different incorrect result.

comment:11 by Jacob, 12 years ago

milestone: 1.0

Milestone 1.0 deleted

Note: See TracTickets for help on using tickets.
Back to Top