Code

Opened 9 years ago

Closed 9 years ago

#479 closed defect (fixed)

[patch] Implement timezone-formats in date-formatting

Reported by: sune.kirkeby@… Owned by: adrian
Component: Database layer (models, ORM) Version:
Severity: normal Keywords:
Cc: jforcier@… Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

In db/typecast timezone-information from the database is discarded. This is bad, which this patch fixes.
Also, the timezone-related formatting characters were NotImplemented in utils/dateformat, this patch also fixes that.
(I assigned this to database-wrapper, but it's just as much template-system...)

Attachments (8)

django-tzinfo.patch (4.9 KB) - added by sune.kirkeby@… 9 years ago.
django-timesince.patch (1.1 KB) - added by sune.kirkeby@… 9 years ago.
Fix timesince to take timezones into account
django-timesince.2.patch (1.1 KB) - added by sune.kirkeby@… 9 years ago.
timezone.patch (6.2 KB) - added by sune.kirkeby@… 9 years ago.
Patch with changes from both the other patch and bugfixes
timezone.2.patch (6.6 KB) - added by sune.kirkeby@… 9 years ago.
Bugfix update of previous patch
dateformat.patch (13.7 KB) - added by Sune Kirkeby <sune.kirkeby@…> 9 years ago.
New, improved and working patch for timezone-formatting in dateformat
dateformat.py (1.4 KB) - added by Sune Kirkeby <sune.kirkeby@…> 9 years ago.
tzinfo.py (1.5 KB) - added by Sune Kirkeby <sune.kirkeby@…> 9 years ago.

Download all attachments as: .zip

Change History (23)

Changed 9 years ago by sune.kirkeby@…

Changed 9 years ago by sune.kirkeby@…

Fix timesince to take timezones into account

Changed 9 years ago by sune.kirkeby@…

Changed 9 years ago by sune.kirkeby@…

Patch with changes from both the other patch and bugfixes

comment:1 Changed 9 years ago by adrian

  • Status changed from new to assigned

comment:2 Changed 9 years ago by anonymous

  • Cc sune.kirkeby@… added

Changed 9 years ago by sune.kirkeby@…

Bugfix update of previous patch

comment:3 Changed 9 years ago by jforcier@…

  • Cc jforcier@… added

comment:4 Changed 9 years ago by Sune Kirkeby <sune.kirkeby@…>

  • Cc sune.kirkeby@… removed
  • Summary changed from [patch] Keep timezones in db/typecast, implement timezone-formats in date-formatting to [patch] Implement timezone-formats in date-formatting

The timezone-parsing patch is bad. Very bad. It breaks the unittests for all but PostgreSQL, and datetime comparisons break spectacularly because datetime is brain-dead when it comes to timezones (Unsafe datetime comparisons).

I worked out a much better way of getting what I want (timezone-formatting in dateformat); in dateformat I
assume that the timezone is local, unless explicitly given. This works beautifully.

Also, I implemented all formats except "Swatch internet time", implemented backslash-escaping of format-chars
and added unittests.

Changed 9 years ago by Sune Kirkeby <sune.kirkeby@…>

New, improved and working patch for timezone-formatting in dateformat

comment:5 Changed 9 years ago by adrian

Sune -- Just to be clear, which patches are currently valid? Just the last two (timezone.2.patch and dateformat.patch)? Apply those two, and that's it?

comment:6 Changed 9 years ago by Sune Kirkeby <sune.kirkeby@…>

Nope, just dateformat.patch, apply that and all should be good :)

comment:7 Changed 9 years ago by Sune Kirkeby <sune.kirkeby@…>

Except,

+        t = time.mktime(now)

in timesince should be

+        t = now.timetuple()

Sorry.

comment:8 Changed 9 years ago by adrian

I get the following unit-test errors after applying this patch (and renaming the variable from my_birthday to sunes_birthday).

'dateformat' module: API test failed
====================================
Code: "format(sunes_birthday, 'O')"
Line: 21
Expected: "'+0100'\n"
Got: "'+1800'\n"

'dateformat' module: API test failed
====================================
Code: "format(sunes_birthday, 'r')"
Line: 25
Expected: "'Sat, 7 Jul 1979 22:00:00 +0100'\n"
Got: "'Sat, 7 Jul 1979 22:00:00 +1800'\n"

'dateformat' module: API test failed
====================================
Code: "format(sunes_birthday, 'U')"
Line: 37
Expected: "'300445200'\n"
Got: "'304117200'\n"

'dateformat' module: API test failed
====================================
Code: "format(sunes_birthday, 'Z')"
Line: 49
Expected: "'3600'\n"
Got: "'64800'\n"

'dateformat' module: API test failed
====================================
Code: "format(summertime, 'O')"
Line: 54
Expected: "'+0200'\n"
Got: "'+1900'\n"

'dateformat' module: API test failed
====================================
Code: "format(wintertime, 'O')"
Line: 58
Expected: "'+0100'\n"
Got: "'+1800'\n"

comment:9 Changed 9 years ago by adrian

In [969]: Lightly refactored django.utils.dateformat to make it use less code. Also integrated some of Sune's improvements from the #479 patch, including support for backslash escaping. Also vastly improved template docs for the {% now %} tag

comment:10 Changed 9 years ago by Sune Kirkeby <sune.kirkeby@…>

django.utils.dateformat should be reloaded after the timezone-fidgeting in the test-module,
since the UTC-offsets are calculated when dateformat is first imported. D'oh!

I'll attach a new test-module now.

/me hopes this works

Changed 9 years ago by Sune Kirkeby <sune.kirkeby@…>

comment:11 Changed 9 years ago by adrian

I get the exact same unit-test errors now...Any ideas?

comment:12 Changed 9 years ago by adrian

Also, your original patch had changes to core/db/typecasts.py...Those should probably still be applied, no?

comment:13 Changed 9 years ago by Sune Kirkeby <sune.kirkeby@…>

Also, your original patch had changes to core/db/typecasts.py...Those should probably still be applied, no?

I'd rather not; I learned that having timezones come back from the database is going to break a lot of client code; because datetime barfs if you try mixing timezone-aware and timezone-naive datetimes. And, as long as the database returns datetimes in settings.TIME_ZONE (which I know postgresql does) the defaulting to LocalTimezone in dateformat and timesince is going to do the right thing; even accounting for DST.

comment:14 Changed 9 years ago by Sune Kirkeby <sune.kirkeby@…>

I get the exact same unit-test errors now...Any ideas?

This time I actually tried running the tests with env TZ=America/Chicago, which gave me the same errors you see. And, it seems a reload of tzinfo is not enough for some reason, so I removed the pre-calculation of timezone-offsets in tzinfo, which does the trick for me. I'll attach the new django.utils.tzinfo.

And the reload-trick in the latest dateformat-tests is not needed with this tzinfo.

Changed 9 years ago by Sune Kirkeby <sune.kirkeby@…>

comment:15 Changed 9 years ago by adrian

  • Resolution set to fixed
  • Status changed from assigned to closed

(In [992]) Fixed #479 -- Implemented time-zone formats in dateformat. Thanks, Sune

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.