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

time.strptime without a year fails on Feb 29 #58365

Closed
MartinMorrison mannequin opened this issue Feb 29, 2012 · 22 comments
Closed

time.strptime without a year fails on Feb 29 #58365

MartinMorrison mannequin opened this issue Feb 29, 2012 · 22 comments
Assignees
Labels
stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@MartinMorrison
Copy link
Mannequin

MartinMorrison mannequin commented Feb 29, 2012

BPO 14157
Nosy @abalkin, @pitrou, @vstinner, @hynek, @phmc
Files
  • strptime-on-leap-years.diff
  • strptime-restore-1900.diff
  • strptime-restore-1900-v2.diff
  • 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 = 'https://github.com/hynek'
    closed_at = <Date 2012-05-14.17:51:17.762>
    created_at = <Date 2012-02-29.11:57:47.973>
    labels = ['type-bug', 'library']
    title = 'time.strptime without a year fails on Feb 29'
    updated_at = <Date 2016-02-29.18:04:45.606>
    user = 'https://bugs.python.org/MartinMorrison'

    bugs.python.org fields:

    activity = <Date 2016-02-29.18:04:45.606>
    actor = 'Sriram Rajagopalan'
    assignee = 'hynek'
    closed = True
    closed_date = <Date 2012-05-14.17:51:17.762>
    closer = 'pitrou'
    components = ['Library (Lib)']
    creation = <Date 2012-02-29.11:57:47.973>
    creator = 'Martin.Morrison'
    dependencies = []
    files = ['25301', '25580', '25581']
    hgrepos = []
    issue_num = 14157
    keywords = ['patch', 'needs review']
    message_count = 21.0
    messages = ['154621', '154642', '154659', '154661', '158207', '158926', '159692', '159736', '160358', '160359', '160360', '160637', '160638', '160639', '160644', '160646', '160648', '160649', '261002', '261005', '261015']
    nosy_count = 11.0
    nosy_names = ['belopolsky', 'pitrou', 'vstinner', 'Arfrever', 'swalker', 'polymorphm', 'python-dev', 'hynek', 'Martin.Morrison', 'pconnell', 'Sriram Rajagopalan']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue14157'
    versions = ['Python 2.7', 'Python 3.2', 'Python 3.3']

    @MartinMorrison
    Copy link
    Mannequin Author

    MartinMorrison mannequin commented Feb 29, 2012

    time.strptime without a year fails on Feb 29 with:

    >>> time.strptime("Feb 29", "%b %d")
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "/usr/lib/python2.6/_strptime.py", line 454, in _strptime_time
        return _strptime(data_string, format)[0]
      File "/usr/lib/python2.6/_strptime.py", line 440, in _strptime
        datetime_date(year, 1, 1).toordinal() + 1
    ValueError: day is out of range for month

    This is due to the use of "1900" as the default year when parsing. It would be nice to have an optional "defaults" keyword argument to the strptime function that can be used to override the defaults, thus allowing leap year dates to be parsed without specifying the date.

    (Note: the code in question attempted to set the year *after* the parse so that ultimately there is a valid struct_time, but since the parse never succeeds, this can't work).

    @MartinMorrison MartinMorrison mannequin added type-bug An unexpected behavior, bug, or error stdlib Python modules in the Lib dir labels Feb 29, 2012
    @abalkin
    Copy link
    Member

    abalkin commented Feb 29, 2012

    This strikes me as an implementation artifact. There is no reason for time.strptime() to validate date triplets. Applications that require valid dates can use datetime.strptime(). I suggest changing time.strptime() specification to match POSIX strptime(). My understanding is that POSIX only requires field by field range checking (%d range 01 to 31, %m range 01 to 12) and not full structure validation. This would be consistent with the way leap seconds are currently treated:

    >>> time.strptime('60', '%S')[5]
    60

    @abalkin abalkin self-assigned this Feb 29, 2012
    @abalkin abalkin added type-feature A feature request or enhancement and removed type-bug An unexpected behavior, bug, or error labels Feb 29, 2012
    @swalker
    Copy link
    Mannequin

    swalker mannequin commented Feb 29, 2012

    I'm seeing this when a year *is* specified with Python 2.6 and 2.7:

    import time
    time.strptime("20090229T184823Z", "%Y%m%dT%H%M%SZ")
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "/usr/lib/python2.6/_strptime.py", line 454, in _strptime_time
        return _strptime(data_string, format)[0]
      File "/usr/lib/python2.6/_strptime.py", line 440, in _strptime
        datetime_date(year, 1, 1).toordinal() + 1
    ValueError: day is out of range for month
    
    
    import datetime
    datetime.datetime.strptime("20090229T184823Z", "%Y%m%dT%H%M%SZ")
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "/usr/lib/python2.6/_strptime.py", line 440, in _strptime
        datetime_date(year, 1, 1).toordinal() + 1
    ValueError: day is out of range for month

    @swalker
    Copy link
    Mannequin

    swalker mannequin commented Feb 29, 2012

    I'm an idiot; nevermind my comment. The original date was bogus.

    @hynek
    Copy link
    Member

    hynek commented Apr 13, 2012

    The point isn’t that time.strptime validates dates but that it uses datetime internally:

    julian = datetime_date(year, month, day).toordinal() - \
                          datetime_date(year, 1, 1).toordinal() + 1

    Is it worth to reimplement this functionality? It strikes easier to me to just use a different year if year is undefined and date == Feb 29.

    @hynek
    Copy link
    Member

    hynek commented Apr 21, 2012

    I gave it a shot, doesn’t look like a hack to me, what do you think?

    @abalkin
    Copy link
    Member

    abalkin commented Apr 30, 2012

    This is a bit of a hack, but seems to get the work done. Does anyone have any objections to committing?

    @pitrou
    Copy link
    Member

    pitrou commented Apr 30, 2012

    Fine with me.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented May 10, 2012

    New changeset f2ea7505c0d7 by Antoine Pitrou in branch '3.2':
    Issue bpo-14157: Fix time.strptime failing without a year on February 29th.
    http://hg.python.org/cpython/rev/f2ea7505c0d7

    New changeset a5a254e8a291 by Antoine Pitrou in branch 'default':
    Issue bpo-14157: Fix time.strptime failing without a year on February 29th.
    http://hg.python.org/cpython/rev/a5a254e8a291

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented May 10, 2012

    New changeset 69d407b016c1 by Antoine Pitrou in branch '2.7':
    Issue bpo-14157: Fix time.strptime failing without a year on February 29th.
    http://hg.python.org/cpython/rev/69d407b016c1

    @pitrou
    Copy link
    Member

    pitrou commented May 10, 2012

    Patch committed and pushed, thank you!

    @MartinMorrison
    Copy link
    Mannequin Author

    MartinMorrison mannequin commented May 14, 2012

    This solution has some very undesirable properties - namely that Mar 1st is now less than Feb 29th!

    It seems like the correct follow up fix would be to adjust the date of the returned struct_time back to 1900. The struct_time object doesn't have the validation issue, so this works fine. This pair of fixes then nicely circumvents the intermediate datetime object's checking, while providing a consistent end result.

    @pitrou
    Copy link
    Member

    pitrou commented May 14, 2012

    That's a good point, thank you. Hynek, do you want to provide a new patch?

    @pitrou pitrou reopened this May 14, 2012
    @hynek
    Copy link
    Member

    hynek commented May 14, 2012

    On it.

    I wonder whether it causes trouble that we return an invalid time_struct down the road?

    @hynek
    Copy link
    Member

    hynek commented May 14, 2012

    I have added a restoration including a short explanation + a regression test.

    @hynek
    Copy link
    Member

    hynek commented May 14, 2012

    Small adjustments to the test as discussed in IRC.

    @hynek hynek self-assigned this May 14, 2012
    @hynek hynek added type-bug An unexpected behavior, bug, or error and removed type-feature A feature request or enhancement labels May 14, 2012
    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented May 14, 2012

    New changeset 83598eb0d761 by Antoine Pitrou in branch '3.2':
    Followup to issue bpo-14157: respect the relative ordering of values produced by time.strptime().
    http://hg.python.org/cpython/rev/83598eb0d761

    New changeset d1c0b57aeb1b by Antoine Pitrou in branch 'default':
    Followup to issue bpo-14157: respect the relative ordering of values produced by time.strptime().
    http://hg.python.org/cpython/rev/d1c0b57aeb1b

    New changeset cbc9dc1c977e by Antoine Pitrou in branch '2.7':
    Followup to issue bpo-14157: respect the relative ordering of values produced by time.strptime().
    http://hg.python.org/cpython/rev/cbc9dc1c977e

    @pitrou
    Copy link
    Member

    pitrou commented May 14, 2012

    Thanks, this should be fine now.

    @pitrou pitrou closed this as completed May 14, 2012
    @polymorphm
    Copy link
    Mannequin

    polymorphm mannequin commented Feb 29, 2016

    $ python
        Python 3.5.1 (default, Dec  7 2015, 12:58:09) 
        [GCC 5.2.0] on linux
        Type "help", "copyright", "credits" or "license" for more information.
        >>> 
        >>> 
        >>> 
        >>> import time
        >>> 
        >>> time.strptime("Feb 29", "%b %d")
        time.struct_time(tm_year=1900, tm_mon=2, tm_mday=29, tm_hour=0, tm_min=0, tm_sec=0, tm_wday=0, tm_yday=60, tm_isdst=-1)
        >>> 
        >>> 
        >>> import datetime
        >>> 
        >>> datetime.datetime.strptime("Feb 29", "%b %d")
        Traceback (most recent call last):
          File "<stdin>", line 1, in <module>
          File "/usr/lib/python3.5/_strptime.py", line 511, in _strptime_datetime
            return cls(*args)
        ValueError: day is out of range for month

    @SriramRajagopalan
    Copy link
    Mannequin

    SriramRajagopalan mannequin commented Feb 29, 2016

    datetime.strptime() uses the return value of _strptime() [ which returns 1900 for 29th Feb without an year ] and eventually ends up calling datetime_new()->check_date_args() [ datetimemodule.c ] with 29th Feb 1900 and eventual failure.

    Should we enhance check_date_args to take a year_dont_care flag and validate the input year argument only if it is explicitly passed?

    @SriramRajagopalan
    Copy link
    Mannequin

    SriramRajagopalan mannequin commented Feb 29, 2016

    Opened bpo-26460 for fixing the leap day bug in datetime.datetime.strptime()

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @gpshead
    Copy link
    Member

    gpshead commented Mar 1, 2024

    I wonder whether it causes trouble that we return an invalid time_struct down the road?

    turns out the answer is yes. #70647 =)

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants