661 tzinfos could be None in call to parse #681
Conversation
|
@ParseThis This PR came in just before we added a PR template with further instructions. I edited your comment into the new PR template. There are a few housekeeping items left. You mention that this is a WIP - is there more to do other than the changelog and AUTHORS.md? |
dateutil/parser/_parser.py
Outdated
| @@ -1115,16 +1115,18 @@ def _build_tzinfo(self, tzinfos, tzname, tzoffset): | |||
| tzdata = tzinfos(tzname, tzoffset) | |||
| else: | |||
| tzdata = tzinfos.get(tzname) | |||
| # user opted to not pass in tzinfos | |||
There was a problem hiding this comment.
I don't think "user opted not to pass in tzinfos" is correct here. I believe this line is only reached if you explicitly pass something that returns None.
dateutil/parser/_parser.py
Outdated
| # or tzinfos is set to None e.g { 'BRST' : None} | ||
| # don't punish them. | ||
| if tzdata is None: | ||
| return |
There was a problem hiding this comment.
I prefer to avoid multiple returns if possible. I would probably refactor this by changing line 1124 below to:
if isinstance(tzdata, datetime.tzinfo) or tzdata is None:
dateutil/test/test_parser.py
Outdated
| @@ -748,6 +747,11 @@ def testUnspecifiedDayFallbackFebLeapYear(self): | |||
| self.assertEqual(parse("Feb 2008", default=datetime(2010, 1, 31)), | |||
| datetime(2008, 2, 29)) | |||
|
|
|||
| def testTzinfoCouldbeNone(self): | |||
There was a problem hiding this comment.
Can you also add a separate test that checks that lambda *args: None also works without raising an error?
| @@ -1160,7 +1162,7 @@ def _build_tzaware(self, naive, res, tzinfos): | |||
| warnings.warn("tzname {tzname} identified but not understood. " | |||
| "Pass `tzinfos` argument in order to correctly " | |||
| "return a timezone-aware datetime. In a future " | |||
| "version, this raise an " | |||
| "version, this will raise an " | |||
|
@pganssle Nothing else, since its my first PR I wanted to sure of the standard here. Thanks for editing the comments! Everything else you mentioned makes sense and will be added. |
This commit allows the user to specify None when the dont care about the time zone information in the date time parse.
5ba764e to
5b98933
Compare
|
This looks good to me. I cleaned up the history a bit, but once the tests pass I'll merge this. Thanks for your work on this! |
Summary of changes
Return
Noneif the user would like to ignore the timezone in the date parse.Fix typo in warning in
_build_tzawareThis is a WIP on #661.
Closes #661
Pull Request Checklist