Skip to content
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

Add datetime.time.strptime and datetime.date.strptime #41431

Open
josh-sf mannequin opened this issue Jan 12, 2005 · 50 comments · May be fixed by #5578
Open

Add datetime.time.strptime and datetime.date.strptime #41431

josh-sf mannequin opened this issue Jan 12, 2005 · 50 comments · May be fixed by #5578
Assignees
Labels
extension-modules C modules in the Modules dir type-feature A feature request or enhancement

Comments

@josh-sf
Copy link
Mannequin

josh-sf mannequin commented Jan 12, 2005

BPO 1100942
Nosy @amauryfa, @abalkin, @vstinner, @devdanzin, @berkerpeksag, @soltysh, @matrixise, @vajrasky, @matheusportela, @pganssle
PRs
  • WIP: bpo-1100942: Add datetime.time.strptime and datetime.date.strptime #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:

    assignee = None
    closed_at = None
    created_at = <Date 2005-01-12.14:53:01.000>
    labels = ['extension-modules', 'type-feature', '3.8']
    title = 'Add datetime.time.strptime and datetime.date.strptime'
    updated_at = <Date 2018-08-20.19:46:06.268>
    user = 'https://bugs.python.org/josh-sf'

    bugs.python.org fields:

    activity = <Date 2018-08-20.19:46:06.268>
    actor = 'p-ganssle'
    assignee = 'none'
    closed = False
    closed_date = None
    closer = None
    components = ['Extension Modules']
    creation = <Date 2005-01-12.14:53:01.000>
    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']

    @josh-sf
    Copy link
    Mannequin Author

    josh-sf mannequin commented Jan 12, 2005

    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.

    @josh-sf josh-sf mannequin added extension-modules C modules in the Modules dir labels Jan 12, 2005
    @alanvgreen
    Copy link
    Mannequin

    alanvgreen mannequin commented Jan 25, 2005

    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:

    1. 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.)

    2. 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?

    @josh-sf
    Copy link
    Mannequin Author

    josh-sf mannequin commented Feb 2, 2005

    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?

    @josh-sf
    Copy link
    Mannequin Author

    josh-sf mannequin commented Feb 17, 2005

    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.

    @sonderblade
    Copy link
    Mannequin

    sonderblade mannequin commented Jun 5, 2007

    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.

    @tiran tiran added type-feature A feature request or enhancement labels Jan 6, 2008
    @devdanzin
    Copy link
    Mannequin

    devdanzin mannequin commented Feb 14, 2009

    Patch needs updating.

    @devdanzin devdanzin mannequin changed the title datetime.strptime constructor added Add datetime.time.strptime and datetime.date.strptime Feb 14, 2009
    @amauryfa
    Copy link
    Member

    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

    @AlexanderBelopolsky
    Copy link
    Mannequin

    AlexanderBelopolsky mannequin commented Apr 20, 2010

    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.

    @AlexanderBelopolsky
    Copy link
    Mannequin

    AlexanderBelopolsky mannequin commented Apr 20, 2010

    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 abalkin self-assigned this May 25, 2010
    @abalkin
    Copy link
    Member

    abalkin commented May 31, 2010

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

    @mdickinson
    Copy link
    Member

    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
    Copy link
    Member

    abalkin commented Jun 9, 2010

    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 abalkin added the easy label Jun 9, 2010
    @abalkin
    Copy link
    Member

    abalkin commented Aug 18, 2010

    Is anyone still interested in moving this forward?

    @abalkin
    Copy link
    Member

    abalkin commented Jan 11, 2011

    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
    Copy link
    Member

    abalkin commented Jun 13, 2012

    Bumping priority to get this in before beta.

    @JuarezBochi
    Copy link
    Mannequin

    JuarezBochi mannequin commented Jul 19, 2012

    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."

    @JuarezBochi
    Copy link
    Mannequin

    JuarezBochi mannequin commented Jul 20, 2012

    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.

    @JuarezBochi
    Copy link
    Mannequin

    JuarezBochi mannequin commented Jul 20, 2012

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

    @JuarezBochi
    Copy link
    Mannequin

    JuarezBochi mannequin commented Jul 20, 2012

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

    It's still missing the modifications proposed on msg107402.

    @BreamoreBoy
    Copy link
    Mannequin

    BreamoreBoy mannequin commented Nov 4, 2012

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

    @abalkin
    Copy link
    Member

    abalkin commented Nov 4, 2012

    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
    Copy link
    Member

    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

    @jafo
    Copy link
    Mannequin

    jafo mannequin commented Mar 19, 2013

    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?

    @abalkin
    Copy link
    Member

    abalkin commented Apr 3, 2014

    Is this documentation still valid?

    +.. staticmethod:: date.strptime(date_string, format)
    +
    + Return a :class:`date` corresponding to *date_string*, parsed according to
    + *format*. This is equivalent to ``date(*(time.strptime(date_string,
    + format)[0:3]))``.

    I understand that the latest patch includes checking for time fields in date format.

    @soltysh
    Copy link

    soltysh commented Apr 4, 2014

    Alexander yes it's correct. It's checking for time part in date.strptime and for time part in time.strptime. The only problem I came into is that when passing 0 hours or 0 minutes into date.strptime it won't raise an exception, though doc explicitly says: "(...) ValueError is raised if the date string (...) the time part is nonzero". So I'm not sure whether this is enough or should I add additional checks if time part was set?

    @abalkin
    Copy link
    Member

    abalkin commented Apr 4, 2014

    If datetime.date.strptime(date_string, format) validates format, then it is *not* equivalent to date(*(time.strptime(date_string, format)[0:3])), is it?

    @soltysh
    Copy link

    soltysh commented Apr 4, 2014

    You're right, I'll change this description removing 'This is equivalent...' sentence from description. I guess the same applies to
    time.strptime as well.

    @soltysh
    Copy link

    soltysh commented Apr 9, 2014

    Sorry it took me that long - but I'm finally attaching fixed patch. I've also checked it again current default branch and updated descriptions accordingly.

    @BreamoreBoy
    Copy link
    Mannequin

    BreamoreBoy mannequin commented May 4, 2015

    @alexander as the datetime expert could you get this committed in time for 3.5?

    @soltysh
    Copy link

    soltysh commented May 9, 2015

    I've just double checked, this patch applies cleanly to latest tip. I wouldn't mind having this reviewed and merged.

    @berkerpeksag
    Copy link
    Member

    datetime.strptime is a classmethod, but the new date.strptime and time.strptime methods are staticmethods. I think we should make the new methods classmethods too.

    @soltysh
    Copy link

    soltysh commented May 11, 2015

    Berker per your comment updated patch changing those two new methods (namely date.strptime and time.strptime) to be classmethod and not staticmethods.

    @matrixise
    Copy link
    Member

    Here is an updated version (for 3.6) of the patch of maciej.szulik.

    I have executed all the tests.

    Please, could you review this patch.

    Thank you

    @abalkin
    Copy link
    Member

    abalkin commented Aug 24, 2016

    This does not look right:

    +.. classmethod:: time.strptime(date_string, format)
    +
    + Return a :class:`time` corresponding to *date_string, parsed according to
    + *format*. :exc:`ValueError` is raised if the date string and format can't be
    + parsed by `time.strptime`, if it returns a value which isn't a time tuple,
    + or if the time part is nonzero.
    ^^^^^^^^^

    @abalkin abalkin added the 3.7 (EOL) end of life label Sep 14, 2016
    @abalkin abalkin removed their assignment Sep 14, 2016
    @matrixise
    Copy link
    Member

    belopolsky, could you tell me what it is wrong with the doc about time.strptime ?

    @abalkin
    Copy link
    Member

    abalkin commented Oct 24, 2016

    Shouldn't "time part" underlined in my previous note be "date part" instead? Also, does non-zero mean non-empty?

    @abalkin
    Copy link
    Member

    abalkin commented Oct 24, 2016

    Never mind the second question.

    @matheusportela
    Copy link
    Mannequin

    matheusportela mannequin commented Jul 22, 2017

    The patch is broken against Python 3.7. I'll try working on it.

    @matheusportela
    Copy link
    Mannequin

    matheusportela mannequin commented Jul 22, 2017

    Also, may I move this issue to a GitHub PR?

    @matrixise
    Copy link
    Member

    I have updated the patch for 3.8, create a PR and fixed the documentation of _strptime._strptime, because this function returns a 3-tuple and not a 2-tuple as indicated in its comment.

    Thank you

    @matrixise matrixise added 3.8 only security fixes and removed 3.7 (EOL) end of life labels Feb 7, 2018
    @matrixise
    Copy link
    Member

    Hello, just a small reminder for this issue and the PR ;-) when you have time

    @vstinner
    Copy link
    Member

    I removed the easy keyword from this issue: it's open since 2005, it has 12 patches and 1 PR attached, and a lot of discussion.

    @zooba zooba removed the easy label Jul 28, 2018
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 9, 2022
    @abalkin
    Copy link
    Member

    abalkin commented Feb 8, 2023

    I have updated the patch for 3.8, create a PR ..

    @matrixise, I don't see a PR linked to this issue.

    @abalkin abalkin self-assigned this Feb 8, 2023
    @abalkin abalkin removed the 3.8 only security fixes label Feb 8, 2023
    @abalkin
    Copy link
    Member

    abalkin commented Feb 8, 2023

    Found it: GH-5578.

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    extension-modules C modules in the Modules dir type-feature A feature request or enhancement
    Projects
    Development

    Successfully merging a pull request may close this issue.

    9 participants