[WIP] Calendar: Make calendar period repeat properly over daylight saving time switch
The test demonstrate the problem I am trying to fix.
I don't understand why ... :
- PresencePeriod needs a custom implementation of getNextPeriodicalDate ?
- we have DateUtils.addToDate ? at least, it's implementation of adding one day is not consistent with what I expect regarding DST switch (it just adds 24 hours).
tests at https://nexedi.erp5.net/test_result_module/20160706-39A894FE are passing, but this implementation is ugly.
( maybe finish attempt of testing this method from jerome/erp5@e639f87d )
Some more notes, instead of this buggy addToDate and complex periodicity, we could just use timedelta and pytz that works just fine for this case:
>>> import DateTime, datetime, pytz >>> DateTime.DateTime(2016, 3, 27, 0, 0, 0, 'Europe/Paris') DateTime('2016/03/27 00:00:00 Europe/Paris') >>> DateTime.DateTime(2016, 3, 27, 0, 0, 0, 'Europe/Paris') + 1 DateTime('2016/03/28 01:00:00 Europe/Paris') # wrong, hour changed >> DateTime.DateTime(datetime.datetime(2016, 3, 27).replace(tzinfo=pytz.timezone('Europe/Paris')) + datetime.timedelta(days=1)) DateTime('2016/03/28 00:00:00 Europe/Paris')
Why do you say addToDate is buggy?
I would prefer to use it everywhere, as date operation are usually wrongly handled when manually calculated.
Why do you say addToDate is buggy?
I have not investigated why, but using addToDate(next_start_date, second=second_duration) made that test fail.
I can see it behaves like this:
>>> import DateTime >>> from Products.ERP5Type.DateUtils import addToDate >>> addToDate(DateTime.DateTime(2016, 3, 27, 0, 0, 0, 'Europe/Paris'), day=1) DateTime('2016/03/28 01:00:00 Europe/Paris')
So adding one day just adds 24 hours. I don't know if it is a bug, or the expected behavior of addToDate, but it behaves just the same of adding a float to a DateTime instance to add a given number of seconds.
Recently, I realized we can use pytz and standard python datetime + timedelta for date arithmetics.
DateTime as a
asdatetimemethod that returns a datetime with a pytz timezone and DateTime constructor accepts a datetime as parameter ( an example here ).
I had a quick chat with @vpelletier , we both share a crazy dream that one day we can stop using DateTime and use datetime instead. This change is enormous and there is no transition plan, so let's not talk about it here and now, but from this discussion I think we agree that we could already use timedelta for date arithmetics without any change.
in other words, we can already do in restricted python:
import datetime, DateTime return DateTime.DateTime((DateTime.DateTime(2016, 3, 27, 0, 0, 0, 'Europe/Paris').asdatetime() + datetime.timedelta(days=1)))
and it returns
2016/03/28 00:00:00 Europe/Paris.
stop using DateTime
Yes, definitely stopping to use the implementation known as DateTime (maybe we could implement something monkey-patching the whole DateTime class to migrate to saner arythmetic while allowing backward compatibility during the transition phase...).
and use datetime instead
Or something better if it exists. datatime has the enormous advantage of being standard python and being sanely designed. The API is simple enough to read the doc without getting distracted, while exposing enough (?) of the time-keeping complexity. Especially that timedelta do not mix days and seconds (aka, a day is not 24 hours, as Jerome illustrated).
It does have the shortcoming (?) of ignoring leapseconds - or, more accurately, of being consistent with unix timestamp way of just ignoring leap seconds altogether and expecting a repeating second.
About this discussion of dropping Products.ERP5Type.DateUtils.addToDate in favor of datetime + timedelta, I relalised addToDate supports adding months and year. Once again, I don't know if addToDate's behavior in "corner cases" such as adding one month to 31 January or adding one year to 29 February are really specified / documented.
Back on this merge request, I realise my patch works fine when we create a DateTime with explicit timezone (for example Europe/Paris), but does not support the case where zope runs in an environment where machine has a DST timezone, we just call DateTime() or use DateTime field without timezone and Datetime does its magic with time.tzname to automatically find the correct local timezone depending on DST.
Since this magic happens at import time of DateTime module depending on locale setup of the machine, it is not easy to simulate in tests. Also, I am not convinced that running zope to have implicit timezones is the best choice and it is probably better to have it running in UTC (even if all the production instances I am working with still have an implicit timezone for historical reason).
So I will not try to support this case of implicit timezone, because it seems hard to test reliably and because I am not sure this implicit timezone is a good thing in the first place and because the project where we discovered this bug do not need really it.
( FYI, it seems most of our testnodes are currently running Europe/Paris timezone )