-
Notifications
You must be signed in to change notification settings - Fork 506
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Deprecate tz format #606
Deprecate tz format #606
Conversation
I will take a look today at this when I get home. |
9c5f40e
to
3b7b558
Compare
@@ -1328,7 +1326,7 @@ Check the daylight limit. | |||
'EDT' | |||
>>> datetime(2003, 10, 26, 0, 59, tzinfo=tz).tzname() | |||
'EDT' | |||
>>> datetime(2003, 10, 26, 1, 00, tzinfo=tz).tzname() | |||
>>> datetime(2003, 10, 26, 2, 00, tzinfo=tz).tzname() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this indicates we're not testing the doctests. Probably we should figure out how to mock out now in doctests so that we can actually get passing doctests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apart from the suggestion, LGTM
'is now deprecated; support for parsing this format ' + | ||
'will be removed in future versions. It is recommended ' + | ||
'that you switch to a standard format like the GNU ' + | ||
'TZ variable format.', tz.DeprecatedTzFormatWarning) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: Include link to the GNU TZ variable format either withing dateutil documentation or to the format specification.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good idea, and one I considered. I think it is in the tzstr documentation. I'm a bit hesitant to add it here, since I'm afraid of it going out of date. I'll merge this for now, we can tweak the message later.
Fixes #595.
This is a release blocker, so we should get this out ASAP.
CC: @pablogsal