-
-
Notifications
You must be signed in to change notification settings - Fork 30k
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
gh-41431: Add datetime.time.strptime()
and datetime.date.strptime()
#120752
base: main
Are you sure you want to change the base?
Conversation
def _strptime_datetime_date(cls, data_string, format="%a %b %d %Y"): | ||
"""Return a date instance based on the input string and the | ||
format string.""" | ||
tt, _, _ = _strptime(data_string, format) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we check the format here? The previous pull request used a C function for that, but that obviously wouldn't work if the C implementation isn't available. I think it would be a bit weird if strftime()
worked, but strptime()
didn't.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we do need to check that this doesn't have any non-date components in it, no? We might need to push that into _strptime
, though. We could check that it returns all 0s, but then things like 2024-01-01T00:00
will fail to raise an exception.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't strptime()
be able to parse the output produced by strftime()
? I would prefer if both raised an error for the format or neither, but checking for the fallback date or time seems fine to me.
>>> datetime.date(9999, 12, 31).strftime("%Y-%m-%dT%H:%M:%S")
'9999-12-31T00:00:00'
>>> datetime.time(23, 59, 59).strftime("%Y-%m-%dT%H:%M:%S")
'1900-01-01T23:59:59'
datetime.time.strptime
and datetime.date.strptime
datetime.time.strptime()
and datetime.date.strptime()
This comment was marked as resolved.
This comment was marked as resolved.
Doc/library/datetime.rst
Outdated
time tuple. See also :ref:`strftime-strptime-behavior` and | ||
:meth:`date.fromisoformat`. | ||
|
||
.. note |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it fine to have this note twice in the documentation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think documentation experts will know better than me, but I think there's a way to define a substitution for this, like this:
|format-note|
.. |format-note| replace::
If *format* specifies a day of month without a year a :exc:`DeprecationWarning` is emitted. This is to avoid a quadrenial leap year bug in code seeking to parse only a month and day, as the default year used when unspecified in the format is not a leap year. Such *format* values may raise an error in Python 3.15. The workaround is to always include a year in your *format*. If parsing *date_string* values that do not have a year, explicitly add a year that is a leap year before parsing:
.. doctest::
>> from datetime import date
>> date_string = "02/29"
>> when = date.strptime(f"{date_string};1984", "%m/%d;%Y") # Avoids leap year bug.
>> when.strftime("%B %d") # doctest: +SKIP
'February 29'
.. versionadded:: 3.14
Then you put |format-note|
in the other place as well.
Two other options:
- move this to a small fragment file (seems like overkill), and use an
..include
directive with it. - put the note in one place, then link to it in the other place (e.g. "See the note on :func:
datetime.strptime
").
I don't think there's a simple way to do it with a parameter, though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was just wondering if it was a problem it was mentioned twice. Do I link to datetime.strptime
or is this fine?
I skip my turn :-) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I had a pending review that I forgot to send. Here are some comments!
Misc/NEWS.d/next/Library/2024-06-19-19-53-42.gh-issue-41431.gnkUc5.rst
Outdated
Show resolved
Hide resolved
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great so far! I have some suggestions here mostly about the tests.
I'm also working on some hypothesis based tests for this, because there are a couple of properties that we can easily and usefully test here, specifically:
dt == dt.strptime(dt.strftime(format), format) == dt
my_datetime.strftime(format_with_just_dates).date() == my_datetime.date().strftime(format_with_just_dates)
my_datetime.strftime(format_with_just_times).time() == my_datetime.time().strftime(format_with_just_times)
I think the key will just be getting a good hypothesis strategy that generates the format strings.
def _strptime_datetime_date(cls, data_string, format="%a %b %d %Y"): | ||
"""Return a date instance based on the input string and the | ||
format string.""" | ||
tt, _, _ = _strptime(data_string, format) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we do need to check that this doesn't have any non-date components in it, no? We might need to push that into _strptime
, though. We could check that it returns all 0s, but then things like 2024-01-01T00:00
will fail to raise an exception.
format) | ||
got = date.strptime(string, format) | ||
self.assertEqual(expected, got) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably test that we fail when passed time components:
def test_strptime_date_with_time_error(self): | |
formats = [ | |
('12', '%H'), | |
('03', '%M'), | |
('12', '%S'), | |
('12:30:14.531', '%H:M:%S.%f'), | |
('2004-02-03 04:11:23.145', '%Y-%m-%d %H:%M:%S.%f'), | |
('2004-02 12', '%Y-%m %H'), | |
] | |
for dtstr, format in formats: | |
with self.subTest(format=format): | |
with self.assertRaises(ValueError): | |
self.theclass.strptime(dtstr, format) |
with self.assertRaises(ValueError): strptime("-2400", "%z") | ||
with self.assertRaises(ValueError): strptime("-000", "%z") | ||
with self.assertRaises(ValueError): strptime("z", "%z") | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to above, we should test to see that there are no date components in the string.
def test_strptime_date_with_time_error(self): | |
formats = [ | |
('2004', '%Y'), | |
('02', '%m'), | |
('13', '%d'), | |
('2004-02-13', '%Y-%m-%d'), | |
('2004-02-03 04:11:23.145', '%Y-%m-%d %H:%M:%S.%f'), | |
('2004-02 12', '%Y-%m %H'), | |
] | |
for dtstr, format in formats: | |
with self.subTest(format=format): | |
with self.assertRaises(ValueError): | |
self.theclass.strptime(dtstr, format) |
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 |
Co-authored-by: Paul Ganssle <1377457+pganssle@users.noreply.github.com>
📚 Documentation preview 📚:
library/datetime.html