Add timezone to generated DTSTART when UNTIL is aware#693
Conversation
|
|
||
| from freezegun import freeze_time | ||
|
|
||
| import pytest |
|
|
||
|
|
||
| @freeze_time(datetime(2018,3,6,5,36,tzinfo=tz.tzutc())) | ||
| def testDtstartTimeZone(self): |
There was a problem hiding this comment.
There are some PEP 8 violations in this test. Arguments need spaces between them, e.g.
datetime(2018, 3, 6, 5, 36, tzinfo=tz.tzutc()) and rrule(freq=HOURLY, until=datetime(2018, 3, 6, 8, 0, ...)
|
|
||
| @freeze_time(datetime(2018,3,6,5,36,tzinfo=tz.tzutc())) | ||
| def testDtstartTimeZone(self): | ||
| rule_without_dtstart = rrule(freq=HOURLY,until=datetime(2018,3,6,8,0,tzinfo=tz.tzutc())) |
There was a problem hiding this comment.
I would prefer tz.UTC be used instead of tz.tzutc().
| def testDtstartTimeZone(self): | ||
| rule_without_dtstart = rrule(freq=HOURLY,until=datetime(2018,3,6,8,0,tzinfo=tz.tzutc())) | ||
| rule_with_dtstart = rrule(freq=HOURLY,dtstart=datetime(2018,3,6,5,36,tzinfo=tz.tzutc()),until=datetime(2018,3,6,8,0,tzinfo=tz.tzutc())) | ||
| assert list(rule_without_dtstart) == list(rule_with_dtstart) |
There was a problem hiding this comment.
You can leave this as is, but generally in the unittest-style tests, you use self.assertEqual instead of bare assert statements. In this case I'm just going to convert this over to a pytest style test before merging, though, so you can leave this as is.
See gh issue #652 - before this the generated DTSTART was always naive, even if UNTIL had a time zone, leading to error cases.
|
@absreim I did the cleanup myself on this one because I figured I would want to do the unittest -> pytest conversion anyway. Thanks for your contribution! It's obvious you put a lot of thought into this! |
Summary of changes
Adds a feature when initializing an rrule with a specified UNTIL but without an explicitly specified DTSTART so that the generated DTSTART takes on the time zone of the UNTIL. Without this feature in that situation, rrule would raise a ValueError because the generated DTSTART is not time zone aware.
Closes #652
Pull Request Checklist