-
-
Notifications
You must be signed in to change notification settings - Fork 33.3k
gh-121237: Add %:z directive to datetime.strptime
#136961
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
Changes from 3 commits
e842519
221ba98
57098db
ed63eed
f499dee
75c1bd1
bb92e82
fa4d19c
4bc5181
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -406,34 +406,41 @@ def test_offset(self): | |
| (*_, offset), _, offset_fraction = _strptime._strptime("-013030.000001", "%z") | ||
| self.assertEqual(offset, -(one_hour + half_hour + half_minute)) | ||
| self.assertEqual(offset_fraction, -1) | ||
| (*_, offset), _, offset_fraction = _strptime._strptime("+01:00", "%z") | ||
| self.assertEqual(offset, one_hour) | ||
| self.assertEqual(offset_fraction, 0) | ||
| (*_, offset), _, offset_fraction = _strptime._strptime("-01:30", "%z") | ||
| self.assertEqual(offset, -(one_hour + half_hour)) | ||
| self.assertEqual(offset_fraction, 0) | ||
| (*_, offset), _, offset_fraction = _strptime._strptime("-01:30:30", "%z") | ||
| self.assertEqual(offset, -(one_hour + half_hour + half_minute)) | ||
| self.assertEqual(offset_fraction, 0) | ||
| (*_, offset), _, offset_fraction = _strptime._strptime("-01:30:30.000001", "%z") | ||
| self.assertEqual(offset, -(one_hour + half_hour + half_minute)) | ||
| self.assertEqual(offset_fraction, -1) | ||
| (*_, offset), _, offset_fraction = _strptime._strptime("+01:30:30.001", "%z") | ||
| self.assertEqual(offset, one_hour + half_hour + half_minute) | ||
| self.assertEqual(offset_fraction, 1000) | ||
| (*_, offset), _, offset_fraction = _strptime._strptime("Z", "%z") | ||
donBarbos marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| self.assertEqual(offset, 0) | ||
| self.assertEqual(offset_fraction, 0) | ||
| for directive in ("%z", "%:z"): | ||
| (*_, offset), _, offset_fraction = _strptime._strptime("+01:00", | ||
| directive) | ||
| self.assertEqual(offset, one_hour) | ||
| self.assertEqual(offset_fraction, 0) | ||
| (*_, offset), _, offset_fraction = _strptime._strptime("-01:30", | ||
| directive) | ||
| self.assertEqual(offset, -(one_hour + half_hour)) | ||
| self.assertEqual(offset_fraction, 0) | ||
| (*_, offset), _, offset_fraction = _strptime._strptime("-01:30:30", | ||
| directive) | ||
| self.assertEqual(offset, -(one_hour + half_hour + half_minute)) | ||
| self.assertEqual(offset_fraction, 0) | ||
| (*_, offset), _, offset_fraction = _strptime._strptime("-01:30:30.000001", | ||
| directive) | ||
| self.assertEqual(offset, -(one_hour + half_hour + half_minute)) | ||
| self.assertEqual(offset_fraction, -1) | ||
| (*_, offset), _, offset_fraction = _strptime._strptime("+01:30:30.001", | ||
| directive) | ||
| self.assertEqual(offset, one_hour + half_hour + half_minute) | ||
| self.assertEqual(offset_fraction, 1000) | ||
|
|
||
| def test_bad_offset(self): | ||
| with self.assertRaises(ValueError): | ||
| _strptime._strptime("-01:30:30.", "%z") | ||
| for directive in ("%z", "%:z"): | ||
| with self.assertRaises(ValueError): | ||
| _strptime._strptime("-01:30:30.", directive) | ||
| with self.assertRaises(ValueError): | ||
| _strptime._strptime("-01:30:30.1234567", directive) | ||
| with self.assertRaises(ValueError): | ||
| _strptime._strptime("-01:30:30:123456", directive) | ||
donBarbos marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| with self.assertRaises(ValueError): | ||
| _strptime._strptime("-0130:30", "%z") | ||
| with self.assertRaises(ValueError): | ||
| _strptime._strptime("-01:30:30.1234567", "%z") | ||
| with self.assertRaises(ValueError): | ||
| _strptime._strptime("-01:30:30:123456", "%z") | ||
| with self.assertRaises(ValueError) as err: | ||
| _strptime._strptime("-01:3030", "%z") | ||
| self.assertEqual("Inconsistent use of : in -01:3030", str(err.exception)) | ||
|
Comment on lines
446
to
448
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The error message should be improved for this case, i.e. lack of colons, (see my previous review for the current state) and tested in this pr IMO. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. After thinking about it, I realized that the “unconverted data remains” error does not happen with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I understand why you suggested that (because it seems to indicate a lack of colons), but it's actually a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oof. This does seem symmetrical with the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For reference, it currently raises: I have a good feeling people will complain, as the message isn't helpful and also makes it seem like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I accepted all suggestions, including trying to carefully write a hack for this case, and taking advantage of the fact that the error was actually different and in other place, I clarified what the error was. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I suspect you are right, but also I am kind of hoping that at some point in the future we will do a teardown rewrite of |
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| Support ``%:z`` directive for :meth:`~datetime.datetime.strptime` and | ||
| :meth:`~datetime.time.strptime`. Patch by Semyon Moroz. | ||
donBarbos marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
Uh oh!
There was an error while loading. Please reload this page.