Skip to content

Conversation

@absreim
Copy link
Contributor

@absreim absreim commented Apr 18, 2018

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

  • Changes have tests
  • Authors have been added to AUTHORS.md
  • News fragment added in changelog.d. See CONTRIBUTING.md for details


from freezegun import freeze_time

import pytest
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't used.



@freeze_time(datetime(2018,3,6,5,36,tzinfo=tz.tzutc()))
def testDtstartTimeZone(self):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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()))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@pganssle pganssle changed the title Issue 652 Add timezone to generated DTSTART when UNTIL is aware Apr 19, 2018
@pganssle pganssle merged commit 2d81f0a into dateutil:master Apr 20, 2018
@pganssle
Copy link
Member

@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!

@pganssle pganssle mentioned this pull request May 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants