Can not set completion date on milestone due to timezone daylight saving

[Felix Schwarz <felix.schwarz@…>]

If I want to set a completion date for a milestone very close to the actual time, I get the warning message: "Warning: Completion date may not be in the future". For example at '04/27/09 15:30:00' I can not put a completion date of a milestone which is '04/27/09 15:00:00'.

I think this is something closely related to trac's/Python's datetime handling with timezones (the culprit is probably 'trac.util.datefmt.parse_date').

This is my setup:

• Trac 0.11.4 (seeing this with 0.11stable as well)
• Python 2.3 (also checked with Python 2.5)
• pytz is installed
• server timezone (default_timezone in trac.ini) is set to utc
• in the user preferences a different timezone was stored, in that timezone daylight saving is currently active (e.g. Europe/Berlin?)

What happens is that parse_date returns a date with the non-dst timezone. Therefore the computed time is off by one hour and compared to UTC it looks like it is a date in the future (because trac only deducts 1h instead of 2h in Europe/Berlin? (CEST -> GMT+2).

This does not happen if pytz is not installed because trac's timezones don't imply any daylight saving settings.

You can add a workaround for the specific issue if you just remove the check for future days. However this still affects plugin writers.

I think the correct way to deal with this is to use the normalize()/localize() method of a pytz tzinfo object on the generated datetime. I think something like that should be added (branches/0.11-stable/trac/util/datefmt.py#L233​):

if not hasattr(tzinfo, 'localize'):
    # This is a tzinfo define by trac which don't have to deal with dst
    dt = datetime(*(tm[0:6] + (0, tzinfo)))
else:
    # We need to detect daylight saving correctly - see #...
    dt = tzinfo.localize(datetime(*tm[0:6]))

[cboos]

Yes, supporting the DST correctly is definitely needed (see also #5895).

As the ​pytz documentation states:

using the tzinfo argument of the standard datetime constructors ‘’does not work’’ with pytz for many timezones.

So we should rather find a way to consistently use localize/normalize in to_datetime/to_timestamp and related places.

[Felix Schwarz <felix.schwarz@…>]

I think this should be a fairly simply change if we assume that all datetime instances - once built - contain the correct timezone. So I think the only functions to change are to_datetime and parse_date in datefmt.py.

[Jappie <jappie@…>]

I think this should be a fairly simply change if we assume that all datetime
instances - once built - contain the correct timezone. So I think the only
functions to change are to_datetime and parse_date in datefmt.py.

Is there an indication to if and when such fix will be made?

[Jappie <jappie@…>]

Replying to Jappie <jappie@…>:

Is there an indication to if and when such fix will be made?

Sorry for not giving additional information!

trac: 0.11.5
python: 2.6.2-r1
pytz: 2009j
timezone: Europe/Amsterdam?

I have the same problem as reported by Felix Schwarz. The patch Felix Schwarz sugests works perfectly for me.
I haven't encountered a related issue in Trac yet, so I cannot verify if the second solution will work.

[rblank]

The pytz docstring for tzinfo.localize() is quite explicit about this:

Convert naive time to local time. This method should be used to construct localtimes, rather than passing a tzinfo argument to a datetime constructor.

So I think Felix' suggestion is correct: we should always construct "naive" datetime objects (i.e. without a tzinfo), then pass them to tzinfo.localize(). I have a slightly more elegant solution than the proposed patch. I also have a test case that fails with the current implementation, and passes when tzinfo.localize() is used. Patch follows.

[rblank]

Use tzinfo.localize() when constructing datetime objects.

[rblank]

Patch applied in [8831].

[rblank]

Following the consistent failures on Tim's buildbot, I have investigated this some more, and I can reproduce the issue by switching my timezone to PST. I'm working on a fix.

[rblank]

I was a bit too enthusiastic with my changes from [8831]. Only the calls to the datetime constructor must be wrapped in tzinfo.localize(), not the calls to now() or fromtimestamp().

I didn't notice the failure, because the one test case that tested it was using exactly my timezone. The issue is fixed in [8857], with an additional test to avoid this special case.