Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#23756 closed Bug (invalid)

Fall DST change breaks timezone.py make_aware

Reported by: Mark Dineen Owned by: nobody
Component: Utilities Version: 1.7
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

make_aware calls localize with DST=None, which breaks with a AmbiguousTimeError when given naive date strings that fall within the 'fall-back' hour. Example: AmbiguousTimeError: 2014-11-02 01:47:44.554654

Celety project has a very elegant solution at https://github.com/celery/django-celery/blob/master/djcelery/utils.py#L54-L62

I implemented this in my project to clear errors resulting from this years DST change. When faces with making an assumption, it calculates both localized aware dates (with and without DST) and takes the min value.

Change History (7)

comment:1 Changed 5 years ago by Tim Graham

Easy pickings: unset
Triage Stage: UnreviewedAccepted

There were many failures on Jenkins on November 2, e.g.

======================================================================
ERROR [0.003s]: test_different_timezones (utils_tests.test_timesince.TimesinceTests)
When using two different timezones.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/jenkins/workspace/django-coverage/tests/utils_tests/test_timesince.py", line 101, in test_different_timezones
    now_tz = timezone.make_aware(now, timezone.get_default_timezone())
  File "/home/jenkins/workspace/django-coverage/django/utils/timezone.py", line 361, in make_aware
    return timezone.localize(value, is_dst=None)
  File "/home/jenkins/workspace/django-coverage/tests/.env/local/lib/python2.7/site-packages/pytz/tzinfo.py", line 349, in localize
    raise AmbiguousTimeError(dt)
AmbiguousTimeError: 2014-11-02 01:59:24.072611

comment:2 Changed 5 years ago by Aymeric Augustin

This is done on purpose according to the Zen of Python: "in the face of ambiguity, refuse the temptation to guess."

Celery's solution is to assume that naive datetimes are in UTC. We can't adopt this solution for make_aware because it takes a timezone argument which can have a value other than UTC.

Always assuming the earliest date may work for your use case, but it may be considered a silent data corruption bug for other use cases, so it isn't an assumption we can enforce in the framework.

comment:3 in reply to:  2 Changed 5 years ago by Mark Dineen

Replying to aaugustin:

This is done on purpose according to the Zen of Python: "in the face of ambiguity, refuse the temptation to guess."

Celery's solution is to assume that naive datetimes are in UTC. We can't adopt this solution for make_aware because it takes a timezone argument which can have a value other than UTC.

Always assuming the earliest date may work for your use case, but it may be considered a silent data corruption bug for other use cases, so it isn't an assumption we can enforce in the framework.

Okay, without specifying a solution, I'd like to highlight that the line below causes problems every time in my code.

https://github.com/django/django/blob/master/django/utils/timezone.py#L361

Can be reproduced with any app using tz, admin, any date field and a date in the affected range.

comment:4 Changed 5 years ago by Aymeric Augustin

Yes, the implementation was specifically designed to raise an exception on ambiguous inputs.

If you have a use case where it's important to enter datetimes in the ambiguous period, then it's up to you to implement a widget that deals with the ambiguity. Django doesn't ship one because there isn't a commonly accepted UI for dealing with this.

comment:5 Changed 5 years ago by Tim Graham

Resolution: invalid
Status: newclosed

I see this is documented, so I think ticket can be closed then.

comment:6 Changed 5 years ago by Jasper Bryant-Greene

I understand the rationale for closing this but wonder whether make_aware could offer an is_dst option to be passed through, allowing the caller of make_aware to resolve the ambiguity if possible?

comment:7 Changed 5 years ago by Carl Meyer

@jbg The main reason I can see not to add the is_dst argument is that (if you're using pytz, which you certainly should be if you care at all about correct timezone handling) timezone.make_aware(value, tz) is simply a different spelling of tz.localize(value), and the latter is actually shorter, and already has the is_dst argument. So if you are using pytz, there's really no reason not to just use tz.localize() instead of timezone.make_aware(). The only real purpose of timezone.make_aware() is to offer semi-correct handling for the no-pytz case, and in that case I don't think there's anything sane we could do with the is_dst argument anyway.

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