Add datetime.time.strptime and datetime.date.strptime

3a57c1f3-e3d5-476c-8e8d-dc193f0a42f4 commented 19 years ago

BPO	1100942
Nosy	@amauryfa, @abalkin, @vstinner, @devdanzin, @berkerpeksag, @soltysh, @matrixise, @vajrasky, @matheusportela, @pganssle
PRs	python/cpython#5578
Files	strptime.diff strptime2.diff: time.strptime and date.strptime as well date-strptime.patch issue1100942.diff issue1100942_pure.diff issue1100942_pure2.diff issue1100942_v4.diff issue1100942_full.patch: Full (merged) patch against current HEAD issue1100942_20140409.patch: Patch fixed against current default branch issue1100942.patch issue1100942-3.6.patch issue1100942_20170722.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields: ```python assignee = None closed_at = None created_at = labels = ['extension-modules', 'type-feature', '3.8'] title = 'Add datetime.time.strptime and datetime.date.strptime' updated_at = user = 'https://bugs.python.org/josh-sf' ``` bugs.python.org fields: ```python activity = actor = 'p-ganssle' assignee = 'none' closed = False closed_date = None closer = None components = ['Extension Modules'] creation = creator = 'josh-sf' dependencies = [] files = ['6427', '6428', '14351', '17599', '26446', '26452', '29117', '33240', '34778', '39345', '44044', '47032'] hgrepos = [] issue_num = 1100942 keywords = ['patch', 'needs review'] message_count = 48.0 messages = ['47516', '47517', '47518', '47519', '47520', '82109', '89650', '103731', '103732', '106805', '106807', '107402', '114247', '126013', '162732', '165882', '165905', '165906', '165929', '174743', '174746', '182336', '184577', '206495', '206498', '206500', '206519', '206717', '215472', '215474', '215491', '215537', '215559', '215843', '242569', '242803', '242809', '242917', '272149', '273568', '279331', '279333', '279334', '298841', '298842', '311783', '313948', '321550'] nosy_count = 22.0 nosy_names = ['jafo', 'guettli', 'amaury.forgeotdarc', 'belopolsky', 'sonderblade', 'alanvgreen', 'vstinner', 'ajaksu2', 'josh-sf', 'cvrebert', 'tiktuk', 'adam-collard', 'westley.martinez', 'berker.peksag', 'maciej.szulik', 'Juarez.Bochi', 'petre', 'matrixise', 'vajrasky', 'Julian.Gindi', 'matheus.v.portela', 'p-ganssle'] pr_nums = ['5578'] priority = None resolution = None stage = 'patch review' status = 'open' superseder = None type = 'enhancement' url = 'https://bugs.python.org/issue1100942' versions = ['Python 3.8'] ```

Linked PRs

gh-120752

3a57c1f3-e3d5-476c-8e8d-dc193f0a42f4 commented 19 years ago

Alllow creating new datetime objects by parsing date strings.

datetime already has strftime, so adding strptime is logical.

The new constructor is equivalent to datetime(*(time.strptime(date_string, format)[0:6])).

Patch includes documentation and unit test.

53132983-aa65-4413-8058-cf6944a83c8e commented 19 years ago

Logged In: YES user_id=1174944

This patch will be welcomed by all of that have had to write "datetime(*(time.strptime(date_string, format)[0:6]))".

I don't understand the C API well enough to check if reference counts are handled properly, but otherwise the implementation looks straight forward.

Documentation looks good and the test passes on my machine.

Two suggestions:

In the time module, the strptime() function's format parameter is optional. For consistency's sake, I'd expect datetime.strptime()'s format parameter also to be optional. (On the other hand, the default value for the format is not very useful.)
Since strftime is supported by datetime.time, datetime.date and datetime.datetime, I'd also expect strptime to be supported by all three classes. Could you add that now, or would it be better to do it as a separate patch?

3a57c1f3-e3d5-476c-8e8d-dc193f0a42f4 commented 19 years ago

Logged In: YES user_id=1194964

Regarding support by datetime.time and datetime.date, if a date component or a time component is specified, respectively, do you think that we should raise an exception? or should we just ignore it?

3a57c1f3-e3d5-476c-8e8d-dc193f0a42f4 commented 19 years ago

Logged In: YES user_id=1194964

The first patch has been applied, now just the second needs to be. (strptime2.diff).

That adds support for date and time as well as datetime, as per alanvgreen's suggestion.

cf40402a-f203-4f98-8392-8fca41a67ee0 commented 17 years ago

The patch doesn't apply cleanly anymore, although that was easy to fix. With the patch, I also get a few implicit declaration warnings and a few conflicting type errors. Rearranging the order of the functions solve that. Fixing that makes the code compile. The two new methods seem to work correct, although there should be unit tests.

90baf024-6604-450d-8341-d796fe6858f3 commented 15 years ago

Patch needs updating.

amauryfa commented 15 years ago

Here is an updated patch, with tests.

The only thing that bugs me is the name of the method: date.strptime() seems a bit odd, given that it cannot accept a time part... OTOH 'strptime' refers to the format specification: %Y-%m-%d

5579dc13-9f48-42d1-bb17-9c003ef6fa70 commented 14 years ago

I am +1 for adding these features and I have only one comment on the code:

It is documented in time.strptime() documentation that """ The default values used to fill in any missing data when more accurate values cannot be inferred are (1900, 1, 1, 0, 0, 0, 0, 1, -1). """ http://docs.python.org/dev/py3k/library/time.html#time.strptime

and "datetime.strptime(date_string, format) is equivalent to datetime(*(time.strptime(date_string, format)[0:6]))." according to datetime module documentation.

Thus, datetime.strptime("", "") returning datetime.datetime(1900, 1, 1, 0, 0) is not an implementation detail and there is no need to compute it in time_strptime.

5579dc13-9f48-42d1-bb17-9c003ef6fa70 commented 14 years ago

BTW, it does not bother me that "date.strptime() seems a bit odd, given that it cannot accept a time part." To me "time" in strptime means time specification that may include date, time or even just month. If parsed specification does not fit in date (includes time component), date.strptime fails. There is nothing wrong with it. An alternative would be to make {date,time}.strptime() promiscuous and just drop unneeded components, but that would make these functions less useful because such behavior is simply datetime.strptime(..).{date,time}().

abalkin commented 14 years ago

Does this need to be brought up on python-dev for acceptance?

mdickinson commented 14 years ago

This doesn't appear to be at all controversial; I don't think it's necessary to consult python-dev. (I haven't looked at the patch, though.)

abalkin commented 14 years ago

I have updated Amaury's patch for py3k. I simplified the test for default date values and fixed a documentation nit. (Time fileds are [4:7], not [4:6]). The result is attached as bpo-1100942.diff.

Note that date.strptime accepts some time specs and time.strptime accepts some date specs:

>>> time.strptime('1900', '%Y')
datetime.time(0, 0)
>>> date.strptime('00', '%H')
datetime.date(1900, 1, 1)

This matches the proposed documentation, but I am not sure is desirable.
I am about +0 for making the test more robust by scanning the format string and rejecting date format codes in time.strptime and time format codes in date. This will also allow better diagnostic messages. For example, instead of

>>> date.strptime('01', '%M')
Traceback (most recent call last):
  ..
ValueError: date.strptime value cannot have a time part

we can produce "'%M' is not valid in date format specification."

abalkin commented 14 years ago

Is anyone still interested in moving this forward?

abalkin commented 13 years ago

New patch needed to address the issue of time.strftime() accepting %Y when year is 1900 and other similar oddities. See msg107402 above. Also a patch for datetime.py is needed.

abalkin commented 12 years ago

Bumping priority to get this in before beta.

391f0820-c034-461d-bfee-e91bc3fc01bd commented 12 years ago

I have updated the patches since they were not applying cleanly and included a pure Python implementation that was missing.

It has the same issues that were mentioned on msg107402.

Do you have any suggestions? I'm planning to block the formats that are not allowed and raise Exceptions like suggested before:

>>> date.strptime('01', '%M')
...
"'%M' is not valid in date format specification."

391f0820-c034-461d-bfee-e91bc3fc01bd commented 12 years ago

I've updated my patch with the tests for datetime.time.strptime that were missing and also its pure Python implementation.

The previous diff also had some issues that I've fixed now: duplicated datetime.strptime method definition in c and the pure python docstring state that date.strpdate was a method and not a constructor.

391f0820-c034-461d-bfee-e91bc3fc01bd commented 12 years ago

Sorry. I updated my patch again to fix the exception message for time.strptime in the pure Python version.

391f0820-c034-461d-bfee-e91bc3fc01bd commented 12 years ago

I've updated the patch based on ezio.melotti and berkerpeksag reviews (thanks).

It's still missing the modifications proposed on msg107402.

83d2e70e-e599-4a04-b820-3814bbdb9bef commented 12 years ago

Could someone please review this with a view to getting the patch into the 3.4 code base, thanks.

abalkin commented 12 years ago

Juarez,

Are you planning to implement format validation as you described in msg165882? Without that, date.strptime() is not very useful because it is almost equivalent to datetime.strptime().date().

berkerpeksag commented 11 years ago

New patch attached.

Changes:

Addressed msg107402. I will update the C code if this implementation is correct.
Added more tests
Converted classmethods to staticmethods
Removed doctests
Updated documentation

eacf80b2-9081-4882-a7f5-27e9644c3853 commented 11 years ago

I've tried to test this but v4 doesn't apply cleanly after pure2 is applied, and v4 doesn't include enough to test it (applying v4 only causes test failures).

I reviewed v4 and it looks fine in general. I do see that there are changes in it unrelated to this issue, but they are PEP-8 changes so I'm not objecting, but ideally that would be split out into a separate patch.

I think that this code will incorrectly detect something like '%s %%wall clock' as a date spec because it contains '%w', but strptime would consider that '%' followed by the string 'wall'. A subtle edge case, but worth considering. Maybe it needs to strip out %% first then look for the % sequences? Or perhaps just do the conversion and if the Y/M/D fields are set in then decide that it included a date spec, or if the HMS are set then say that it has the time spec included?

101e49dd-21b6-4cd8-be82-82a32b99a59e commented 10 years ago

I'm interested in taking over and finishing whatever needs to be completed to move this forward. What else needs to be done? It looks like improved tests are needed, but are there any changes needed to the implementation code?