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

document that strptime() does not support the Feb 29 if the format does not contain the year #63575

Closed
MatthewEarl mannequin opened this issue Oct 24, 2013 · 16 comments
Closed
Labels
3.7 (EOL) end of life 3.8 only security fixes docs Documentation in the Doc dir easy stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@MatthewEarl
Copy link
Mannequin

MatthewEarl mannequin commented Oct 24, 2013

BPO 19376
Nosy @brettcannon, @abalkin, @pitrou, @vstinner, @taleinat, @hynek, @phmc, @pganssle, @csabella, @fbidu, @bradengroom
PRs
  • bpo-19376: Added doc. #10243
  • [3.7] bpo-19376: Added doc mentioning datetime.strptime() without a year fails for Feb 29. (GH-10243) #13470
  • 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 = <Date 2019-05-18.20:39:32.055>
    created_at = <Date 2013-10-24.10:02:17.360>
    labels = ['easy', 'type-bug', '3.8', '3.7', 'library', 'docs']
    title = 'document that strptime() does not support the Feb 29 if the format does not contain the year'
    updated_at = <Date 2019-05-21.21:09:57.938>
    user = 'https://bugs.python.org/MatthewEarl'

    bugs.python.org fields:

    activity = <Date 2019-05-21.21:09:57.938>
    actor = 'vstinner'
    assignee = 'docs@python'
    closed = True
    closed_date = <Date 2019-05-18.20:39:32.055>
    closer = 'cheryl.sabella'
    components = ['Documentation', 'Library (Lib)']
    creation = <Date 2013-10-24.10:02:17.360>
    creator = 'Matthew.Earl'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 19376
    keywords = ['patch', 'easy']
    message_count = 16.0
    messages = ['201108', '201114', '201128', '201137', '201139', '201151', '201158', '328780', '328805', '328807', '328907', '328925', '328995', '342815', '343081', '343097']
    nosy_count = 16.0
    nosy_names = ['brett.cannon', 'belopolsky', 'pitrou', 'vstinner', 'taleinat', 'Arfrever', 'swalker', 'docs@python', 'hynek', 'Martin.Morrison', 'pconnell', 'Matthew.Earl', 'p-ganssle', 'cheryl.sabella', 'fbidu', 'bradengroom']
    pr_nums = ['10243', '13470']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue19376'
    versions = ['Python 2.7', 'Python 3.6', 'Python 3.7', 'Python 3.8']

    @MatthewEarl
    Copy link
    Mannequin Author

    MatthewEarl mannequin commented Oct 24, 2013

    datetime.datetime.strptime() without a year fails on Feb 29 with:

    >>> datetime.datetime.strptime("Feb 29", "%b %d")
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "/auto/ensoft-sjc/thirdparty/lib/python3.3/_strptime.py", line 511, in _strptime_datetime
        return cls(*args)
    ValueError: day is out of range for month

    This is because without a year specified the year is assumed to be 1900, which is not a leap year. The underlying _strptime._strptime() function has some munging such that it doesn't itself fail (see bpo-14157):

    >>> _strptime._strptime("Feb 29", "%b %d")
    ((1900, 2, 29, 0, 0, 0, 0, 60, -1, None, None), 0)

    ...however datetime.datetime.__init__() is called with this tuple as *args, causing the validation failure.

    @MatthewEarl MatthewEarl mannequin added stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Oct 24, 2013
    @vstinner
    Copy link
    Member

    I don't think that the issue can be called a bug. If we pick another year (ex: 1904), you cannot compare two datetimes anymore:
    "This solution has some very undesirable properties - namely that Mar 1st is now less than Feb 29th!"
    http://bugs.python.org/issue14157#msg160637

    If you want to handle "Feb 29", add an explicit year.

    This issue is maybe a documentation issue: datetime.datetime.strptime() should warn users that calling datetime.datetime.strptime() without year may fail for Feb 29.

    @brettcannon
    Copy link
    Member

    I agree with Victor: we should document that proper Feb 29/leap year support requires a specified year, else constantly accepting Feb 29 as valid would lead to more errors than fix.

    @vstinner vstinner added the docs Documentation in the Doc dir label Oct 24, 2013
    @vstinner vstinner changed the title datetime.datetime.strptime without a year fails on Feb 29 document that strptime() does not support the Feb 29 if the format does not contain the year Oct 24, 2013
    @pitrou
    Copy link
    Member

    pitrou commented Oct 24, 2013

    Note: the actual explanation is that Feb 29th doesn't exist in *1900* which is the default year in strptime(). The same error happens if you ask for "Feb 30" or "Apr 31", it has nothing to do with leap years specifically.

    In other words, the documentation looks sufficient to me as-is, and adding special wording for this would only make it longer than it should be.

    @vstinner
    Copy link
    Member

    In other words, the documentation looks sufficient to me as-is, and adding special wording for this would only make it longer than it should be.

    Adding a note would not hurt.

    @MatthewEarl
    Copy link
    Mannequin Author

    MatthewEarl mannequin commented Oct 24, 2013

    Out of interest, what's the reason for accepting the time.strptime() version as a bug, but not datetime.datetime.strptime()? Is it that time.strptime() is meant to be a simple parsing from string to tuple (with minimal checks), whereas datetime.datetime.strptime() should represent an actual point in time, therefore extra validation is expected to occur?

    If so I'm happy to either close or add a small note to the docs (I don't mind which.)

    @abalkin
    Copy link
    Member

    abalkin commented Oct 24, 2013

    what's the reason for accepting the time.strptime()
    version as a bug, but not datetime.datetime.strptime()?

    In case of time.strptime(), we have an option of returning (1900, 2, 29, ..) which while not being a valid date, is a valid (time)tuple:

    >>> time.mktime((1900, 2, 29, 0, 0, 0, 0, 0, 0))
    -2203873200.0

    The time module treats 1900-02-29 as 1900-03-01:

    >>> time.mktime((1900, 3, 1, 0, 0, 0, 0, 0, 0))
    -2203873200.0

    Datetime is stricter than that:

    >>> datetime(1900, 2, 29)
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    ValueError: day is out of range for month

    There is no valid datetime value that can reasonably be returned from datetime.strptime('Feb 29', '%b %d').

    @bradengroom
    Copy link
    Mannequin

    bradengroom mannequin commented Oct 29, 2018

    In other words, the documentation looks sufficient to me as-is, and adding special wording for this would only make it longer than it should be.

    I agree with Victor here. It seems like this can be closed. There haven't been any comments from other people hitting this issue for 5 years.

    @vstinner
    Copy link
    Member

    Paul Ganssle: Do you want to work on a PR? Or should we close the issue?

    @taleinat
    Copy link
    Contributor

    This is a great, easy, doc-only issue for a new contributor to handle.

    @taleinat taleinat added easy 3.7 (EOL) end of life 3.8 only security fixes labels Oct 29, 2018
    @fbidu
    Copy link
    Mannequin

    fbidu mannequin commented Oct 30, 2018

    What about adding some more information in the exception message in addition to the docs? I mean, if we're raising an issue because someone inserted Feb 29, we add a note about leap year in the exception message

    @pganssle
    Copy link
    Member

    @victor: You mean a PR to fix the *issue* or a PR to add this to the docs?

    The current behavior is pretty counter-intuitive, particularly because it also fails because of the (relatively) little-known fact that 1900 happens to not be a leap year because it is evenly divisible by 100 but not by 400.

    I think it's pretty simple for end-users to work around this:

    def strptime_smarter(dtstr, fmt):
        try:
            return datetime.strptime(dtstr, fmt)
        except ValueError:
            tt = time.strptime(dtstr, fmt)
            if tt[0:3] == (1900, 2, 29):
                return datetime(1904, *tt[1:6])
            raise

    But this is largely a problem that arises because we don't have any concept of a "partial datetime", see this dateutil issue: dateutil/dateutil#449

    What users want when they do datetime.strptime("Feb 29", "%b %d") is something like (None, 2, 29), but we're both specifying an arbitrary default year and enforcing that the final date be legal. I think the best solution would be to change the default year to 2000 for all dates, but for historical reasons that is just not feasible. :(

    Another option is that we could allow specifying a "default date" from which missing values would be drawn. We have done this in dateutil.parser.parse: https://dateutil.readthedocs.io/en/stable/parser.html#dateutil.parser.parse

    The biggest problem in dateutil is that the default value for "default date" is the current date, which causes many problems with reproducibility. For datetime.strptime, the default value would be datetime(1900, 1, 1), which has none of those same problems.

    Still, adding such a parameter to datetime.strptime seems like a lot of effort to go through to just to make it easier for people to work around this bug in strptime, particularly since in this case you can kinda do the same thing with:

        strptime('1904 ' + dtstr, '%Y %b %d')

    Long-winded carping on about datetime issues aside, I think my final vote is for leaving the behavior as-is and documenting it. Looking at the documentation, the only documentation I see for what happens when you don't have %Y, %m or %d is:

    For time objects, the format codes for year, month, and day should not be used,
    as time objects have no such values. If they’re used anyway, 1900 is substituted
    for the year, and 1 for the month and day.
    

    This only makes sense in the context of strftime. I think for starters we should document the behavior of strptime when no year, month or day are specified. As part of that documentation, we can add a footnote about Feb 29th.

    I can make a PR for this, but as Tal mentions, I think this is a good issue for a first-time contributor, so I'd like to give someone else an opportunity to take a crack at this.

    @taleinat
    Copy link
    Contributor

    I think for starters we should document the behavior of strptime when no year, month or day are specified. As part of that documentation, we can add a footnote about Feb 29th.

    This sounds good to me.

    We could also improve the exception raised in the specific case of Feb 29th; IMO this should be a separate PR.

    @csabella
    Copy link
    Contributor

    New changeset 56027cc by Cheryl Sabella (Abhishek Kumar Singh) in branch 'master':
    bpo-19376: Added doc mentioning datetime.strptime() without a year fails for Feb 29. (GH-10243)
    56027cc

    @taleinat
    Copy link
    Contributor

    New changeset 390d88e by Tal Einat in branch '3.7':
    [3.7] bpo-19376: Added doc mentioning datetime.strptime() without a year fails for Feb 29. (GH-10243)
    390d88e

    @vstinner
    Copy link
    Member

    Thanks Abhishek Kumar Singh! more doc is really helpful, handling date and time is hard :-(

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.7 (EOL) end of life 3.8 only security fixes docs Documentation in the Doc dir easy stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    7 participants