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

datetime.strptime doesn't support %z format ? #50890

Closed
eka mannequin opened this issue Aug 4, 2009 · 22 comments
Closed

datetime.strptime doesn't support %z format ? #50890

eka mannequin opened this issue Aug 4, 2009 · 22 comments
Assignees
Labels
type-feature A feature request or enhancement

Comments

@eka
Copy link
Mannequin

eka mannequin commented Aug 4, 2009

BPO 6641
Nosy @brettcannon, @mdickinson, @abalkin, @merwok
Dependencies
  • bpo-5094: datetime lacks concrete tzinfo implementation for UTC
  • Files
  • issue6641.diff
  • issue6641a.diff
  • issue6641b.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/abalkin'
    closed_at = <Date 2010-06-18.18:49:24.333>
    created_at = <Date 2009-08-04.14:04:36.567>
    labels = ['type-feature']
    title = "datetime.strptime doesn't support %z format ?"
    updated_at = <Date 2010-06-19.04:36:22.073>
    user = 'https://bugs.python.org/eka'

    bugs.python.org fields:

    activity = <Date 2010-06-19.04:36:22.073>
    actor = 'eric.araujo'
    assignee = 'belopolsky'
    closed = True
    closed_date = <Date 2010-06-18.18:49:24.333>
    closer = 'belopolsky'
    components = ['None']
    creation = <Date 2009-08-04.14:04:36.567>
    creator = 'eka'
    dependencies = ['5094']
    files = ['17681', '17699', '17710']
    hgrepos = []
    issue_num = 6641
    keywords = ['patch']
    message_count = 22.0
    messages = ['91256', '91282', '91763', '91780', '106434', '106473', '107329', '107549', '107555', '107797', '107902', '108032', '108033', '108034', '108038', '108039', '108041', '108115', '108116', '108117', '108127', '108166']
    nosy_count = 8.0
    nosy_names = ['brett.cannon', 'mark.dickinson', 'belopolsky', 'eric.araujo', 'eka', 'dmhouse', 'abrown', 'dudologist']
    pr_nums = []
    priority = 'normal'
    resolution = 'accepted'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue6641'
    versions = ['Python 3.2']

    @eka
    Copy link
    Mannequin Author

    eka mannequin commented Aug 4, 2009

    When trying to use datetime.strptime %z directive got an unexpected error.

    >>> from datetime import datetime
    >>> 
    >>> fecha = u'Sun Aug 02 19:01:25 +0000 2009'
    >>> datetime.strptime(fecha, '%a %b %d %H:%M:%S %z %Y')
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "/usr/lib/python2.6/_strptime.py", line 317, in _strptime
        (bad_directive, format))
    ValueError: 'z' is a bad directive in format '%a %b %d %H:%M:%S %z %Y'

    @eka eka mannequin added the type-bug An unexpected behavior, bug, or error label Aug 4, 2009
    @dmhouse
    Copy link
    Mannequin

    dmhouse mannequin commented Aug 4, 2009

    From the documentation from time.strptime() (which acts the same as
    datetime.strptime()):

    "Only the directives specified in the documentation [of time.strftime()]
    are supported. Because strftime() is implemented per platform it can
    sometimes offer more directives than those listed. But strptime() is
    independent of any platform and thus does not necessarily support all
    directives available that are not documented as supported."

    So I think invalid.

    @abrown
    Copy link
    Mannequin

    abrown mannequin commented Aug 20, 2009

    I think this bug is just a doc bug. If you check

    http://docs.python.org/library/datetime.html?highlight=strptime#strftime-behavior

    and

    http://docs.python.org/library/time.html?highlight=strptime#time.strptime

    You can see that the first link lists %z as a valid modifier, while in
    the second link (in footnote 1) it is mentioned as deprecated.

    @dmhouse
    Copy link
    Mannequin

    dmhouse mannequin commented Aug 20, 2009

    Yes and no.

    Firstly, %z isn't listed as deprecated in the documentation of the time
    module's strftime -- although %Z is (note differing case).

    Secondly, I still think the bug is invalid, because the documentation of
    datetime.datetime.strptime says it behaves like time.strptime, whose
    documentation says "only the directives specified in the documentation
    [of strftime()] are supported". Since we're in the time module, that
    reference to strftime() means time.strftime(), which doesn't list %z as
    a directive.

    Finally, there *is* a confusing docs issue, however: the "strftime()
    behaviour" section in the datetime module documentation lists %z as a
    valid directive, whereas it's not listed in time.strftime. Although
    these functions have in theory nothing to do with one another, you would
    in practice expect them to support the same directives.

    Since in fact the footnote in the documentation of time.strftime() says
    %z isn't supported by all ANSI C platforms (despite apparently being
    required by the standard), I suggest that %z be removed from the list of
    allowed modifiers in the "strftime() behaviour" section in the datetime
    module documentation.

    @dudologist
    Copy link
    Mannequin

    dudologist mannequin commented May 25, 2010

    If %z works only in certain circumstances that behaviour should be documented wherever %z is referred to.

    @abalkin
    Copy link
    Member

    abalkin commented May 25, 2010

    If concrete tzinfo subclass is added to datetime module (see bpo-5094), it will become feasible to meaningfully parse timezone information.

    @abalkin abalkin self-assigned this May 25, 2010
    @abalkin abalkin added type-feature A feature request or enhancement and removed type-bug An unexpected behavior, bug, or error labels Jun 8, 2010
    @abalkin
    Copy link
    Member

    abalkin commented Jun 8, 2010

    Attached patch includes bpo-5094 patch.

    @mdickinson
    Copy link
    Member

    It's a little awkward to review this patch independently of the bpo-5094 patch. Can we work on bpo-5094 first, and then come back to this one?

    @abalkin
    Copy link
    Member

    abalkin commented Jun 11, 2010

    On Fri, Jun 11, 2010 at 10:43 AM, Mark Dickinson <report@bugs.python.org> wrote:
    ..

    It's a little awkward to review this patch independently of the bpo-5094 patch.
     Can we work on bpo-5094 first, and then come back to this one?

    Sure. Unfortunately, I think I fixed a few timezone doc warts here
    and did not update 5094. I will fix that.

    @abalkin
    Copy link
    Member

    abalkin commented Jun 14, 2010

    With bpo-5094 patch now committed, I am replacing bpo-6641.diff with a new version that does not include bpo-5094.

    @abalkin abalkin changed the title strptime doesn't support %z format ? datetime.strptime doesn't support %z format ? Jun 14, 2010
    @abalkin
    Copy link
    Member

    abalkin commented Jun 16, 2010

    Mark,

    I am reassigning this to you for a commit review.

    @abalkin abalkin assigned mdickinson and unassigned abalkin Jun 16, 2010
    @mdickinson
    Copy link
    Member

    Doc nit: "When %z directive" -> "When the %z directive"

    The _strptime._strptime docstring is inaccurate: it claim to return a time struct, but actually returns tuple, int; please could you also add docstrings for _strptime_time and _strptime_datetime?

    Spacing in datetimemodule.c: "if( module == NULL)" -> "if (module == NULL)". Also, is there any particular reason for initializing 'result' to NULL? (Or even for using result at all; you could just do "return PyObject_CallMethod(... ").

    I'm mildly distressed by the inability of strptime to parse UTC offsets in the +HH:MM form that str(timezone) produces, but I'm not sure what the solution to that is.

    Otherwise, this all looks good to my non-expert eye.

    @mdickinson mdickinson assigned abalkin and unassigned mdickinson Jun 17, 2010
    @mdickinson
    Copy link
    Member

    Hmm. Hold on a sec; I'm getting test failures...

    @mdickinson
    Copy link
    Member

    In test_datetime:

    ======================================================================
    ERROR: test_strptime (main.TestDateTime)
    ----------------------------------------------------------------------

    Traceback (most recent call last):
      File "Lib/test/test_datetime.py", line 1741, in test_strptime
        dt = strptime("-0500 EST", "%z %Z")
      File "/Users/dickinsm/python/svn/py3k/Lib/_strptime.py", line 483, in _strptime_datetime
        tt, fraction = _strptime(data_string, format)
      File "/Users/dickinsm/python/svn/py3k/Lib/_strptime.py", line 336, in _strptime
        (data_string, format))
    ValueError: time data '-0500 EST' does not match format '%z %Z'

    ======================================================================
    ERROR: test_strptime (main.TestDateTimeTZ)
    ----------------------------------------------------------------------

    Traceback (most recent call last):
      File "Lib/test/test_datetime.py", line 1741, in test_strptime
        dt = strptime("-0500 EST", "%z %Z")
      File "/Users/dickinsm/python/svn/py3k/Lib/_strptime.py", line 483, in _strptime_datetime
        tt, fraction = _strptime(data_string, format)
      File "/Users/dickinsm/python/svn/py3k/Lib/_strptime.py", line 336, in _strptime
        (data_string, format))
    ValueError: time data '-0500 EST' does not match format '%z %Z'

    @abalkin
    Copy link
    Member

    abalkin commented Jun 17, 2010

    issue6641a.diff fixes the nits that Mark found and makes the tests robust with respect to change of timezones. Tested with all timezones avalilable on OSX including TZ="Eire". (Ever heard of Ouagadougou?)

    Parsing RFC 3339's HH:MM format is a separate issue. I am +1 on either adding %:z specifier or simply allow HH:MM for %z.

    @mdickinson
    Copy link
    Member

    LGTM.

    @abalkin
    Copy link
    Member

    abalkin commented Jun 17, 2010

    Committed in r82053.

    @abalkin abalkin closed this as completed Jun 17, 2010
    @abalkin
    Copy link
    Member

    abalkin commented Jun 18, 2010

    Reopening because the patch introduced a regression with respect to datetime subclasses:

    >>> class DT(datetime): pass
    ... 
    >>> DT.strptime('', '')
    datetime.datetime(1900, 1, 1, 0, 0)

    a DT instance expected. Need tests covering subclasses.

    @abalkin abalkin reopened this Jun 18, 2010
    @abalkin
    Copy link
    Member

    abalkin commented Jun 18, 2010

    Attaching issue6641b.diff that fixes subclassing issue. Added tests for datetime subclass only to keep the patch focused. Adding tests for datetime and time subclasses will be natural in bpo-1100942.

    @abalkin
    Copy link
    Member

    abalkin commented Jun 18, 2010

    Replacing issue6641b.diff after fixing some spelling errors. Thanks, Ezio.

    @abalkin
    Copy link
    Member

    abalkin commented Jun 18, 2010

    Committed in r82073.

    @abalkin abalkin closed this as completed Jun 18, 2010
    @merwok
    Copy link
    Member

    merwok commented Jun 19, 2010

    PEP-8 says: “If your public attribute name collides with a reserved keyword, append a single trailing underscore to your attribute name. This is preferable to an abbreviation or corrupted spelling.” e.g. “class_” is used for an HTML class. “(However, notwithstanding this rule, 'cls' is the preferred spelling for any variable or argument which is known to be a class, especially the first argument to a class method.)” I’d like this recommendation to be followed, even if it’s in an internal function.

    @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
    type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants