- 
          
 - 
                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 8 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,42 @@ 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") | ||
| 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) | ||
| (*_, offset), _, offset_fraction = _strptime._strptime("Z", | ||
| directive) | ||
| self.assertEqual(offset, 0) | ||
| self.assertEqual(offset_fraction, 0) | ||
                
      
                  donBarbos marked this conversation as resolved.
               
              
                Outdated
          
            Show resolved
            Hide resolved
         | 
||
| 
     | 
||
| 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,3 @@ | ||
| Support ``%:z`` directive for :meth:`datetime.datetime.strptime`, | ||
| :meth:`datetime.time.strptime` and :func:`time.strptime`. | ||
| Patch by Lucas Esposito and Semyon Moroz. | 
Uh oh!
There was an error while loading. Please reload this page.