-
Notifications
You must be signed in to change notification settings - Fork 509
Add default tzinfo #475
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
Add default tzinfo #475
Conversation
85e09d3
to
a6aae67
Compare
Ping @nealmcb |
Thanks for working on this! So my proposal was implementing something like this: >>> parse("00:00", defaulttz=tzutc()) And it seems that this would be the proposed implementation from this PR a6aae67: >>> from dateutil.utils import default_tzinfo
>>> default_tzinfo(parse("0:00"), tzutc()) I guess that works, and if people get their date objects from somewhere other than |
I guess a practical example would be something that takes two strings to be parsed from some list of meeting times, in which one string has timezone info, and the other doesn't. Both could be parsed, with a default of the |
dateutil/utils.py
Outdated
""" | ||
|
||
dt = datetime.now(tzinfo) | ||
return datetime.combine(dt.date(), time(0, tzinfo=tzinfo)) |
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.
+1 for today(tzinfo)
, but I'm very wary of implementing behavior that differs from same-named stdlib behavior. btw:
>>> tz = pytz.timezone('US/Pacific')
>>> pd.Timestamp.today(tz)
Timestamp('2017-10-14 17:19:04.081315-0700', tz='US/Pacific')
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.
@jbrockmendel Sorry, I miscalculated what would be shown in the diff here, this discussion should be taking place on #474, so I've responded here
if dt.tzinfo is not None: | ||
return dt | ||
else: | ||
return dt.replace(tzinfo=tzinfo) |
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.
lgtm
dateutil/test/test_utils.py
Outdated
from dateutil import utils | ||
from dateutil.utils import within_delta | ||
|
||
from freezegun import freeze_time |
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.
+1. Now if I can figure out #463 to mock tzlocal, we'll make a mockery of this whole endeavor.
@nealmcb Yes, that's not a bad idea. After we settle on the final interface here, I can add it in to the documentation for the parser. |
a6aae67
to
b0167c1
Compare
@nealmcb Added an example in the |
LGTM |
As a partial and possibly more general fix to #94, this adds a
tzinfo
to anydatetime
that doesn't have one, and otherwise leaves it alone.