bpo-34903: For datetime.datetime.strptime(), the leading zero for some two-digit formats is optional #9747

mikegleen · 2018-10-07T05:43:08Z

https://bugs.python.org/issue34903

the-knights-who-say-ni · 2018-10-07T05:43:10Z

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA).

Unfortunately we couldn't find an account corresponding to your GitHub username on bugs.python.org (b.p.o) to verify you have signed the CLA (this might be simply due to a missing "GitHub Name" entry in your b.p.o account settings). This is necessary for legal reasons before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

You can check yourself to see if the CLA has been received.

Thanks again for your contribution, we look forward to reviewing it!

mikegleen · 2018-10-07T06:03:52Z

Sorry for being so lame at creating this pull request. The title should be:
bpo-34903: For datetime.datetime.strptime, the leading zero for some two-digit formats is optional.
Test code:
import datetime
import sys
import unittest

class TestStrptime(unittest.TestCase):
#print(sys.version)

def test_y(self):
    # print('test %y requires leading zero ')
    with self.assertRaises(ValueError):
        newdate = datetime.datetime.strptime('1/2/3 4:5:6', '%d/%m/%y %H:%M:%S')

def test_dmyHMS(self):
    # print('test %d %m %y %H %M %S ')
    newdate = datetime.datetime.strptime('1/2/03 4:5:6', '%d/%m/%y %H:%M:%S')
    self.assertEqual('2003-02-01T04:05:06', newdate.isoformat(timespec='seconds'))

def test_Ij(self):
    # print('test %I %j')
    newdate = datetime.datetime.strptime('2/03 4am:5:6', '%j/%y %I%p:%M:%S')
    self.assertEqual('2003-01-02T04:05:06', newdate.isoformat(timespec='seconds'))

def test_U(self):
    # print('test %U')
    newdate = datetime.datetime.strptime('6/4/03', '%w/%U/%y')
    self.assertEqual('2003-02-01T00:00:00', newdate.isoformat(timespec='seconds'))

def test_W(self):
    # print('test %U')
    newdate = datetime.datetime.strptime('6/4/03', '%w/%W/%y')
    self.assertEqual('2003-02-01T00:00:00', newdate.isoformat(timespec='seconds'))

def test_V(self):
    # print('test %V')
    newdate = datetime.datetime.strptime('6/4/2003', '%u/%V/%G')
    self.assertEqual('2003-01-25T00:00:00', newdate.isoformat(timespec='seconds'))

def show():
print('test %d %m %y %H %M %S ')
print(datetime.datetime.strptime('1/2/03 4:5:6', '%d/%m/%y %H:%M:%S'))
print('test %I %j')
print(datetime.datetime.strptime('2/03 4am:5:6', '%j/%y %I%p:%M:%S'))
print('test %U')
print(datetime.datetime.strptime('6/4/03', '%w/%U/%y'))
print('test %W')
print(datetime.datetime.strptime('6/4/03', '%w/%W/%y'))
print('test %V')
print(datetime.datetime.strptime('6/04/2003', '%u/%V/%G'))

if name == 'main':
ver = sys.version_info
if ver.major != 3 or ver.minor < 6:
print(f'Requires at least 3.6, got {sys.version}')
sys.exit()
unittest.main()

willingc · 2018-10-08T21:57:57Z

@mikegleen You should be able to edit the title on the PR by clicking the Edit button to the right of the title. Please submit the CLA if you haven't yet done so. Thanks.

mikegleen · 2018-10-08T22:07:56Z

Thanks, the title is updated now – I'm a little embarrassed not to have noticed the Edit button. The CLA is in process but there was some confusion as my bugs.python.org userid is not the same as my github userid. I'm told it should may take a couple of days.

…

On Mon, Oct 8, 2018 at 10:58 PM Carol Willing ***@***.***> wrote: @mikegleen <https://github.com/mikegleen> You should be able to edit the title on the PR by clicking the Edit button to the right of the title. Please submit the CLA if you haven't yet done so. Thanks. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#9747 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABy9pn2ZZsXM6Hvo14UXJ4VhGnDRmOTfks5ui8qVgaJpZM4XLsQm> .

willingc · 2018-10-08T22:19:58Z

Thanks for the update @mikegleen. No worries on the button (IMHO the edit button is a bit to far to the right in the GitHub UI). 🍪

augustogoulart · 2018-10-08T23:27:06Z

@mikegleen It would be interesting to have the tests you commented out on this thread added to your PR. Reach me out If you have any questions about that. Thanks!

mikegleen · 2018-10-09T08:50:26Z

Gus, Here's the simple test code. Please let me know if you receive this as I am not familiar with the email address style for replies. Regards, Mike

…

On Tue, Oct 9, 2018 at 12:27 AM Gus Goulart ***@***.***> wrote: @mikegleenIt would be interesting to have the tests you commented out on this thread added to your PR. Reach me out If you have any questions about that. Thanks! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#9747 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABy9phljSvkS20-DT-E0fVnZE0NnAwUPks5ui994gaJpZM4XLsQm> .

augustogoulart · 2018-10-09T10:34:22Z

@mikegleen to make sure your tests are effective, they must be included in the Python test suite. Take a look at /Lib/test and see where you can fit them.
To accomplish that, add your tests to the test file you found, commit the changes and push the new commit to your pull request.

mikegleen · 2018-10-23T06:07:23Z

Gus, I had a look at the existing test code for datetime.strptime and found that it already tests the case of single digit days and months. So it looks like it's just the doc that needs updating. Thanks for pointing me at that. Regards, Mike

…

On Tue, Oct 9, 2018 at 1:35 PM Gus Goulart ***@***.***> wrote: @mikegleen <https://github.com/mikegleen> to make sure your tests are effective, they must be included in the Python test suite. Take a look at /Lib/test and see where you can fit them. To accomplish that, add your tests to the test file you found, commit the changes and push the new commit to your pull request. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#9747 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABy9pvivkWxz6FlV7mw2DIPntTL73x25ks5ujHvngaJpZM4XLsQm> .

willingc · 2018-10-23T13:56:37Z

Hi @mikegleen,

Have you filled out the CLA?

I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA).

Unfortunately we couldn't find an account corresponding to your GitHub username on bugs.python.org (b.p.o) to verify you have signed the CLA (this might be simply due to a missing "GitHub Name" entry in your b.p.o account settings). This is necessary for legal reasons before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

You can check yourself to see if the CLA has been received.

brettcannon · 2019-03-29T21:11:08Z

@mikegleen could you add the test code to the PR?

merge updates

mikegleen · 2019-04-01T14:24:12Z

When Gus Goulart initially mentioned including the test code, I checked datetimetester.py and saw that single digit dates were already tested. I replied about this and did not proceed any further. But now, looking again, I see that the testing is somewhat incidental and it would be better to have a well-defined test of this issue.

So I suggest that I create a separate PR for datetimetester.py, as so much time has passed since the original PR. Are you ok with this?

Mike Gleen

brettcannon · 2019-04-01T19:51:06Z

@mikegleen Up to you. No one will take issue if you add it to this one or create a separate PR. If you do a separate PR then please reference it here so people know where to look for the tests. And I'll leave a Changes Requests review so no one loses track of needing the tests.

brettcannon

Please add tests.

bedevere-bot · 2019-04-01T19:51:20Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

And if you don't make the requested changes, you will be poked with soft cushions!

by strptime. This test pairs with PR 9747 which updates the datetime doc. The corresponding bug report is bpo-34903.

mikegleen · 2019-04-02T13:54:24Z

I've updated datetimetester.py but the pull request failed because (I think) the existing PR (9747) also included the changes to the doc file. So I think I should close 9747 and then again create a PR which will now correctly include changes to both files.

But I thought I should check first. Does this sound right?

mikegleen · 2019-04-04T14:20:09Z

I have made the requested changes; please review again.

bedevere-bot · 2019-04-04T14:20:15Z

Thanks for making the requested changes!

@brettcannon: please review the changes made to this pull request.

brettcannon · 2019-04-26T22:29:05Z

Lib/test/datetimetester.py

@@ -2425,6 +2425,22 @@ def test_strptime(self):
                self.assertEqual(expected, got)

        strptime = self.theclass.strptime
+


There's actually a https://github.com/python/cpython/blob/master/Lib/test/test_strptime.py file for tests relating to strptime.

I am not a huge fan of the test_strptime thing, which tests _strptime directly rather than testing the public API function(s) datetime.strptime and time.strptime.

The reasoning for this is that the tests in datetimetester.py run against the C and datetime implementations per PEP 399, and they are parametrized to be run against subclasses of datetime.

In this case, I think it's unlikely that these tests will be affected by either of these things, but I also don't think the cost is particularly high, so I lean in favor of testing in datetimetester.py rather than test_strptime.py unless there's a compelling reason to do otherwise.

brettcannon · 2019-04-26T22:29:19Z

Lib/test/datetimetester.py

+
+        # bpo-34903: Check that single digit dates and times are allowed.
+        with self.assertRaises(ValueError):
+            # %y does require two digits


Suggested change

# %y does require two digits

# %y does require two digits.

brettcannon · 2019-04-26T22:32:47Z

Lib/test/datetimetester.py

+        with self.assertRaises(ValueError):
+            # %y does require two digits
+            newdate = strptime('1/2/3 4:5:6', '%d/%m/%y %H:%M:%S')
+        inputs = [


Could you provide comments specifying which format option you're specifically testing? Or better yet, provide explicit tests for each format you're testing? (You can keep these real-world examples as well).

brettcannon · 2019-04-26T22:33:12Z

Lib/test/datetimetester.py

+            ('6/4/2003', '%u/%V/%G', '2003-01-25T00:00:00')
+        ]
+        for string, format, target in inputs:
+            newdate = strptime(string, format)


Can you use unittest.subTest()?

bedevere-bot · 2019-04-26T22:33:22Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

update to latest.

updated.

update

update latest

mikegleen · 2019-06-17T10:03:50Z

Replaced by #14149.

note9

e004548

bedevere-bot added the docs Documentation in the Doc dir label Oct 7, 2018

the-knights-who-say-ni added the CLA not signed label Oct 7, 2018

bedevere-bot added the awaiting review label Oct 7, 2018

mikegleen changed the title ~~```~~ bpo-34903: For datetime.datetime.strptime, the leading zero for some two-digit formats is optional. Oct 8, 2018

willingc added the skip news label Oct 8, 2018

brettcannon removed the CLA not signed label Mar 29, 2019

the-knights-who-say-ni added the CLA signed label Mar 29, 2019

brettcannon changed the title ~~bpo-34903: For datetime.datetime.strptime, the leading zero for some two-digit formats is optional.~~ bpo-34903: For datetime.datetime.strptime(), the leading zero for some two-digit formats is optional Mar 29, 2019

Merge branch 'master' of github.com:python/cpython

64de04d

merge updates

brettcannon requested changes Apr 1, 2019

View reviewed changes

bedevere-bot removed the awaiting review label Apr 1, 2019

bedevere-bot added the awaiting changes label Apr 1, 2019

Add tests confirming that single digit dates and times are supported

f8d015d

by strptime. This test pairs with PR 9747 which updates the datetime doc. The corresponding bug report is bpo-34903.

Fix whitespace issues.

7a70073

bedevere-bot added awaiting change review and removed awaiting changes labels Apr 4, 2019

brettcannon self-requested a review April 11, 2019 21:14

brettcannon requested changes Apr 26, 2019

View reviewed changes

bedevere-bot removed the awaiting change review label Apr 26, 2019

bedevere-bot added the awaiting changes label Apr 26, 2019

mikegleen added 7 commits April 27, 2019 07:24

Merge branch 'master' of github.com:python/cpython

04e6ffc

Merge branch 'master' of github.com:python/cpython

57e791f

Merge branch 'master' of github.com:python/cpython

2c50e95

update to latest.

Merge branch 'master' of github.com:python/cpython

b3bf289

updated.

Merge branch 'master' of github.com:python/cpython

2c3f8f3

update

checked out datetimetester.py

ed49b9c

Merge branch 'master' of github.com:python/cpython

17067a6

update latest

mikegleen closed this Jun 17, 2019

mikegleen mentioned this pull request Jun 17, 2019

bpo-34903: For datetime.datetime.strptime(), the leading zero for some two-digit formats is optional. #14149

Merged

		@@ -2425,6 +2425,22 @@ def test_strptime(self):
		self.assertEqual(expected, got)

		strptime = self.theclass.strptime

Uh oh!

bpo-34903: For datetime.datetime.strptime(), the leading zero for some two-digit formats is optional #9747

bpo-34903: For datetime.datetime.strptime(), the leading zero for some two-digit formats is optional #9747

Uh oh!

Conversation

mikegleen commented Oct 7, 2018 • edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

the-knights-who-say-ni commented Oct 7, 2018

Uh oh!

mikegleen commented Oct 7, 2018 • edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

willingc commented Oct 8, 2018

Uh oh!

mikegleen commented Oct 8, 2018 via email

Uh oh!

willingc commented Oct 8, 2018

Uh oh!

augustogoulart commented Oct 8, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mikegleen commented Oct 9, 2018 via email

Uh oh!

augustogoulart commented Oct 9, 2018

Uh oh!

mikegleen commented Oct 23, 2018 via email

Uh oh!

willingc commented Oct 23, 2018

Uh oh!

brettcannon commented Mar 29, 2019

Uh oh!

mikegleen commented Apr 1, 2019

Uh oh!

brettcannon commented Apr 1, 2019

Uh oh!

brettcannon left a comment

Choose a reason for hiding this comment

Uh oh!

bedevere-bot commented Apr 1, 2019

Uh oh!

mikegleen commented Apr 2, 2019

Uh oh!

mikegleen commented Apr 4, 2019

Uh oh!

bedevere-bot commented Apr 4, 2019

Uh oh!

brettcannon Apr 26, 2019

Choose a reason for hiding this comment

Uh oh!

pganssle Jun 16, 2019

Choose a reason for hiding this comment

Uh oh!

brettcannon Apr 26, 2019

Choose a reason for hiding this comment

Uh oh!

brettcannon Apr 26, 2019

Choose a reason for hiding this comment

Uh oh!

brettcannon Apr 26, 2019

Choose a reason for hiding this comment

Uh oh!

bedevere-bot commented Apr 26, 2019

Uh oh!

mikegleen commented Jun 17, 2019 • edited by pganssle Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

mikegleen commented Oct 7, 2018 •

edited by bedevere-bot

Loading

mikegleen commented Oct 7, 2018 •

edited by bedevere-bot

Loading

augustogoulart commented Oct 8, 2018 •

edited

Loading

mikegleen commented Jun 17, 2019 •

edited by pganssle

Loading