taichino / croniter

croniter is a python module to provide iteration for datetime object.
http://github.com/taichino/croniter
387 stars 105 forks source link

Correctly handle DST changes #92

Closed kbrose closed 6 years ago

kbrose commented 6 years ago

See https://github.com/taichino/croniter/issues/91

There are two main changes:

You can verify it by running

import datetime as dt
import pytz
import croniter

s = dt.datetime.strptime('2017-11-01', '%Y-%m-%d')
s = pytz.timezone('US/Eastern').localize(s)
ci = croniter.croniter('0 16 * * *', s)

for i in range(8):
    print(dt.datetime.utcfromtimestamp(next(ci)))

print('-'*19)

s = dt.datetime.strptime('2017-03-08', '%Y-%m-%d')
s = pytz.timezone('US/Eastern').localize(s)
ci = croniter.croniter('0 16 * * *', s)

for i in range(8):
    print(dt.datetime.utcfromtimestamp(next(ci)))

expected output:

2017-11-01 20:00:00
2017-11-02 20:00:00
2017-11-03 20:00:00
2017-11-04 20:00:00
2017-11-05 21:00:00
2017-11-06 21:00:00
2017-11-07 21:00:00
2017-11-08 21:00:00
-------------------
2017-03-08 21:00:00
2017-03-09 21:00:00
2017-03-10 21:00:00
2017-03-11 21:00:00
2017-03-12 20:00:00
2017-03-13 20:00:00
2017-03-14 20:00:00
2017-03-15 20:00:00

The tests on master are currently broken (ref https://github.com/taichino/croniter/pull/89), with test_std_dst failing. With this change, that test now passes, but unfortunately test_std_dst2 fails. However, I am not sure that test_std_dst2 is correctly written. It seems like that test expects the cron string to be specifying the UTC trigger times, but in reality the cron string is interpreted as the local timezone. I'm tempted to rewrite/delete that test. I would like someone else's opinion on that, though, as I'm not 100% sure.

kbrose commented 6 years ago

Looking more at the original issue that spawned test_std_dst2 (https://github.com/taichino/croniter/issues/87), I'm not convinced it was ever fixed. The changes in this PR appear to be behaving correctly in the test case given by OP of #87:

from __future__ import print_function
import pytz
from datetime import datetime, timedelta
from croniter import croniter

tz = pytz.timezone("America/Sao_Paulo")

local_dates = [tz.localize(datetime(2018, 2, 17, 0, 0, 0)),
               tz.localize(datetime(2018, 2, 17, 1, 0, 0)),
               tz.localize(datetime(2018, 2, 17, 2, 0, 0)),
               tz.localize(datetime(2018, 2, 17, 3, 0, 0))]

[print(d, '=>', croniter("0 0 * * *", d).get_next(datetime)) for d in local_dates]
print('-' * 30)
local_dates_5min = [d + timedelta(minutes=5) for d in local_dates]
[print(d, '=>', croniter("0 0 * * *", d).get_next(datetime)) for d in local_dates_5min]
2018-02-17 00:00:00-02:00 => 2018-02-18 00:00:00-03:00
2018-02-17 01:00:00-02:00 => 2018-02-18 00:00:00-03:00
2018-02-17 02:00:00-02:00 => 2018-02-18 00:00:00-03:00
2018-02-17 03:00:00-02:00 => 2018-02-18 00:00:00-03:00
------------------------------
2018-02-17 00:05:00-02:00 => 2018-02-18 00:00:00-03:00
2018-02-17 01:05:00-02:00 => 2018-02-18 00:00:00-03:00
2018-02-17 02:05:00-02:00 => 2018-02-18 00:00:00-03:00
2018-02-17 03:05:00-02:00 => 2018-02-18 00:00:00-03:00
kiorky commented 6 years ago

I validate the changes, its released, thx !

kiorky commented 6 years ago

https://pypi.python.org/pypi/croniter/0.3.20

kbrose commented 6 years ago

Thanks @kiorky !