#7201 closed (fixed)
timeuntil filter misbehaves if my datetime has a timezone
Reported by: | 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)
Change History (15)
comment:1 by , 16 years ago
Component: | Uncategorized → Template system |
---|---|
Keywords: | timeuntil timesince added |
milestone: | → 1.0 |
Triage Stage: | Unreviewed → Accepted |
comment:2 by , 16 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:3 by , 16 years ago
Owner: | removed |
---|---|
Status: | assigned → new |
comment:4 by , 16 years ago
Owner: | set to |
---|---|
Status: | new → assigned |
comment:5 by , 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 , 16 years ago
Ticket #8080 has been created to address the behavior of timesince and timeuntil.
by , 16 years ago
Fixes timezone errors in timeuntil and timesince. Includes tests and documentation updates.
by , 16 years ago
Attachment: | 7201.2.diff added |
---|
Modification of original patch to not assume timezone when none exists.
comment:7 by , 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 , 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 , 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:10 by , 16 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
(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.
Seems to be a clear bug.