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? |
| 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.
| # 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:
| 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?
| "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