Code

Opened 3 years ago

Closed 3 years ago

#16906 closed Cleanup/optimization (fixed)

Avoid strftime when isoformat can do the job

Reported by: aaugustin Owned by: nobody
Component: Core (Other) Version: 1.3
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by aaugustin)

While working on timezone support in Django, the inconsistency in these three functions surprised me: https://code.djangoproject.com/browser/django/trunk/django/db/backends/__init__.py#L714


In fact, r7946 introduced a datetime_safe module that provides a Python implementation of datetime.[date|datetime].strftime for dates before 1900.

However, this is overkill for the purpose of just showing a date or datetime in ISO format (%Y-%m-%d and %Y-%m-%d %H:%M:%S). It makes the code needlessly complicated, sometimes inconistent, and inefficient. The isoformat method achieves the same result, works for any date, and is implemented in C.

Also, some code still uses the strftime function from the standard library. Using isoformat is better because it isn't subject to the 1900 limit.

See attached patch.


For more background, see isoformat_date in http://svn.python.org/view/python/tags/r271/Modules/datetimemodule.c?view=markup. Note that date objects have no __unicode__ and their __str__ just call isoformat, so str(date) or unicode(date) is the same as datetime_safe.date(date.year, date.month, date.day).strftime('%Y-%m-%d'). It's pretty much the same for datetime objects, except that isoformat uses T as a separator, while str and unicode use a space.

Changing date.format('%Y-%m-%d') to date.isoformat() is safe. Changing datetime.format('%Y-%m-%d %H:%M:%S') to datetime.isoformat() requires removing the microseconds and the timezone first, if there is one, so the correct pattern is datetime.replace(microsecond=0, tzinfo=None). This also applies for time objects.

Attachments (1)

16906.patch (7.4 KB) - added by aaugustin 3 years ago.

Download all attachments as: .zip

Change History (4)

Changed 3 years ago by aaugustin

comment:1 Changed 3 years ago by aaugustin

  • Description modified (diff)

comment:2 Changed 3 years ago by lukeplant

  • Triage Stage changed from Unreviewed to Ready for checkin

comment:3 Changed 3 years ago by aaugustin

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

In [16978]:

Fixed #16906 -- Format datetimes with str/unicode instead of strftime where possible: it's faster and it works for all dates.

Also ensured that datetime_safe is used wherever strftime is called on dates/datetimes that may be before 1900.

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.