Skip to content

gh-121237: Add %:z directive to strptime #122142

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.

Sign up for GitHub

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

gh-121237: Add %:z directive to strptime #122142

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3ae4449
gh-121237: Add %:z directive to strptime
LucasEsposito Jul 22, 2024
4a73231
Fix error message
LucasEsposito Jul 22, 2024
43a8c13
Moved some tests utilities from 'test_zipfile/...' to the general 'su…
LucasEsposito Jul 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion Doc/library/datetime.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2538,7 +2538,21 @@ differences between platforms in handling of unsupported format specifiers.
``%G``, ``%u`` and ``%V`` were added.

.. versionadded:: 3.12
``%:z`` was added.
``%:z`` was added for :meth:`~.datetime.strftime`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
``%:z`` was added for :meth:`~.datetime.strftime`
``%:z`` was added for :meth:`~.datetime.strftime`.


.. versionadded:: 3.13
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
.. versionadded:: 3.13
.. versionadded:: 3.14

``%:z`` was added for :meth:`~.datetime.strptime`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
``%:z`` was added for :meth:`~.datetime.strptime`
``%:z`` was added for :meth:`~.datetime.strptime`.


.. warning::
Copy link
Contributor

@zuo zuo Oct 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO, this entire warning is unnecessary, because:

  • There is no need to warn, only to describe certain details (see below). After all, the only inconsistency is that %z is little bit more liberal when it comes to parsing. That's all.
  • The statement that the behavior is unintended is inaccurate. The behavior is intended and sometimes very useful.
  • The recommendation to use %z directive only to parse strings formatted according to %z directive, while using %:z directive for strings formatted according to %:z is unnecessary (IMHO). If somebody needs to accept only :-based notation, they will parse their input with %:z. If somebody needs to be more liberal in respect to the input, they will parse their input with %z. Both approaches are perfectly OK, so there is no need to discourage people from anything here.

On the other hand, it is worth to describe certain details – but, IMHO, a better place to do so is the note (6) in the Technical Detail subsection (below). Namely, I propose to:

  • (1) Replace the following fragment of the %z description:

    For example, ``'+01:00:00'`` will be parsed as an offset of one hour.
In addition, providing ``'Z'`` is identical to ``'+00:00'``.

    with the following text:

    For example, both ``'+010000'`` and ``'+01:00:00'`` will be parsed as an offset
of one hour. In addition, providing ``'Z'`` is identical to ``'+00:00'``.

  • (2) Replace the following fragment of the %:z description:

    Behaves exactly as ``%z``, but has a colon separator added between
hours, minutes and seconds.

    with the following text:

    When used with :meth:`~.datetime.strftime`, behaves exactly as ``%z``, except
that a colon separator is added between hours, minutes and seconds.

When used with :meth:`~.datetime.stpftime`, the UTC offset is *required* to
have a colon as a separator between hours, minutes and seconds.
For example, ``'+01:00:00'`` (but *not* ``'+010000'``) will be parsed as an
offset of one hour. In addition, providing ``'Z'`` is identical to ``'+00:00'``.

  • (3) Do not add this Warning frame at all.


Since version 3.12, when ``%z`` directive is used in :meth:`~.datetime.strptime`,
strings formatted according ``%z`` directive are accepted and parsed correctly,
as well as strings formatted according to ``%:z``.
The later part of the behavior is unintended but it's still kept for backwards
compatibility.
Nonetheless, it's encouraged to use ``%z`` directive only to parse strings
formatted according to ``%z`` directive, while using ``%:z`` directive
for strings formatted according to ``%:z``.

Technical Detail
^^^^^^^^^^^^^^^^
Expand Down
24 changes: 15 additions & 9 deletions Lib/_strptime.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,10 @@ def __init__(self, locale_time=None):
#XXX: Does 'Y' need to worry about having less or more than
# 4 digits?
'Y': r"(?P<Y>\d\d\d\d)",
# "z" shouldn't support colons. Both ":?" should be removed. However, for backwards
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# "z" shouldn't support colons. Both ":?" should be removed. However, for backwards
# "z" might not support colons, if designed from scratch. However, for backwards

Again: the statement that it shoudn't support colons is too strong IMHO. It is perfectly OK, that it supports them.

# compatibility, we must keep them (see gh-121237)
'z': r"(?P<z>[+-]\d\d:?[0-5]\d(:?[0-5]\d(\.\d{1,6})?)?|(?-i:Z))",
':z': r"(?P<colon_z>[+-]\d\d:[0-5]\d(:[0-5]\d(\.\d{1,6})?)?|(?-i:Z))",
'A': self.__seqToRE(self.locale_time.f_weekday, 'A'),
'a': self.__seqToRE(self.locale_time.a_weekday, 'a'),
'B': self.__seqToRE(self.locale_time.f_month[1:], 'B'),
Expand Down Expand Up @@ -254,13 +257,16 @@ def pattern(self, format):
year_in_format = False
day_of_month_in_format = False
while '%' in format:
directive_index = format.index('%')+1
format_char = format[directive_index]
directive_index = format.index('%') + 1
directive = format[directive_index]
if directive == ":":
# Special case for "%:z", which has an extra character
directive += format[directive_index + 1]
processed_format = "%s%s%s" % (processed_format,
format[:directive_index-1],
self[format_char])
format = format[directive_index+1:]
match format_char:
format[:directive_index - 1],
self[directive])
format = format[directive_index + len(directive):]
match directive:
case 'Y' | 'y' | 'G':
year_in_format = True
case 'd':
Expand Down Expand Up @@ -446,16 +452,16 @@ def _strptime(data_string, format="%a %b %d %H:%M:%S %Y"):
week_of_year_start = 0
elif group_key == 'V':
iso_week = int(found_dict['V'])
elif group_key == 'z':
z = found_dict['z']
elif group_key in ('z', 'colon_z'):
z = found_dict[group_key]
if z == 'Z':
gmtoff = 0
else:
if z[3] == ':':
z = z[:3] + z[4:]
if len(z) > 5:
if z[5] != ':':
msg = f"Inconsistent use of : in {found_dict['z']}"
msg = f"Inconsistent use of : in {found_dict[group_key]}"
raise ValueError(msg)
z = z[:5] + z[6:]
hours = int(z[1:3])
Expand Down
12 changes: 12 additions & 0 deletions Lib/test/support/itertools.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# from more_itertools v8.13.0
def always_iterable(obj, base_type=(str, bytes)):
if obj is None:
return iter(())

if (base_type is not None) and isinstance(obj, base_type):
return iter((obj,))

try:
return iter(obj)
except TypeError:
return iter((obj,))
78 changes: 39 additions & 39 deletions Lib/test/test_zipfile/_path/_test_params.py → Lib/test/support/parameterize.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,39 +1,39 @@
import types
import functools

from ._itertools import always_iterable


def parameterize(names, value_groups):
"""
Decorate a test method to run it as a set of subtests.

Modeled after pytest.parametrize.
"""

def decorator(func):
@functools.wraps(func)
def wrapped(self):
for values in value_groups:
resolved = map(Invoked.eval, always_iterable(values))
params = dict(zip(always_iterable(names), resolved))
with self.subTest(**params):
func(self, **params)

return wrapped

return decorator


class Invoked(types.SimpleNamespace):
"""
Wrap a function to be invoked for each usage.
"""

@classmethod
def wrap(cls, func):
return cls(func=func)

@classmethod
def eval(cls, cand):
return cand.func() if isinstance(cand, cls) else cand
import types
import functools
from .itertools import always_iterable
def parameterize(names, value_groups):
"""
Decorate a test method to run it as a set of subtests.
Modeled after pytest.parametrize.
"""
def decorator(func):
@functools.wraps(func)
def wrapped(self):
for values in value_groups:
resolved = map(Invoked.eval, always_iterable(values))
params = dict(zip(always_iterable(names), resolved))
with self.subTest(**params):
func(self, **params)
return wrapped
return decorator
class Invoked(types.SimpleNamespace):
"""
Wrap a function to be invoked for each usage.
"""
@classmethod
def wrap(cls, func):
return cls(func=func)
@classmethod
def eval(cls, cand):
return cand.func() if isinstance(cand, cls) else cand
33 changes: 26 additions & 7 deletions Lib/test/test_strptime.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,12 @@
import sys
from test import support
from test.support import skip_if_buggy_ucrt_strfptime, warnings_helper
from test.support.parameterize import parameterize
from datetime import date as datetime_date

import _strptime


class getlang_Tests(unittest.TestCase):
"""Test _getlang"""
def test_basic(self):
Expand Down Expand Up @@ -354,7 +356,7 @@ def test_julian(self):
# Test julian directives
self.helper('j', 7)

def test_offset(self):
def test_z_directive_offset(self):
Copy link
Contributor

@zuo zuo Oct 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this rename is necessary, considering the surrounding code's method naming convention.

one_hour = 60 * 60
half_hour = 30 * 60
half_minute = 30
Expand All @@ -370,22 +372,39 @@ def test_offset(self):
(*_, offset), _, offset_fraction = _strptime._strptime("-013030.000001", "%z")
self.assertEqual(offset, -(one_hour + half_hour + half_minute))
self.assertEqual(offset_fraction, -1)
(*_, offset), _, offset_fraction = _strptime._strptime("+01:00", "%z")

@parameterize(
Copy link
Contributor

@zuo zuo Oct 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test LGTM. :)

Yet, it would also be nice to extend (appropriately) relevant tests in Lib/test/datetimetester.py (which regard both implementations of the datetime module) – namely:

  • TestDateTime:
    • test_strptime()
  • TestTime:
    • test_strptime_tz()
    • test_strptime_errors()

["directive"],
[
("%z",),
("%:z",),
]
)
def test_iso_offset(self, directive: str):
"""
Tests offset for the '%:z' directive from ISO 8601.
Since '%z' directive also accepts '%:z'-formatted strings for backwards compatibility,
we're testing that here too.
"""
one_hour = 60 * 60
half_hour = 30 * 60
half_minute = 30
(*_, offset), _, offset_fraction = _strptime._strptime("+01:00", directive)
self.assertEqual(offset, one_hour)
self.assertEqual(offset_fraction, 0)
(*_, offset), _, offset_fraction = _strptime._strptime("-01:30", "%z")
(*_, offset), _, offset_fraction = _strptime._strptime("-01:30", directive)
self.assertEqual(offset, -(one_hour + half_hour))
self.assertEqual(offset_fraction, 0)
(*_, offset), _, offset_fraction = _strptime._strptime("-01:30:30", "%z")
(*_, offset), _, offset_fraction = _strptime._strptime("-01:30:30", directive)
self.assertEqual(offset, -(one_hour + half_hour + half_minute))
self.assertEqual(offset_fraction, 0)
(*_, offset), _, offset_fraction = _strptime._strptime("-01:30:30.000001", "%z")
(*_, offset), _, offset_fraction = _strptime._strptime("-01:30:30.000001", directive)
self.assertEqual(offset, -(one_hour + half_hour + half_minute))
self.assertEqual(offset_fraction, -1)
(*_, offset), _, offset_fraction = _strptime._strptime("+01:30:30.001", "%z")
(*_, offset), _, offset_fraction = _strptime._strptime("+01:30:30.001", directive)
self.assertEqual(offset, one_hour + half_hour + half_minute)
self.assertEqual(offset_fraction, 1000)
(*_, offset), _, offset_fraction = _strptime._strptime("Z", "%z")
(*_, offset), _, offset_fraction = _strptime._strptime("Z", directive)
self.assertEqual(offset, 0)
self.assertEqual(offset_fraction, 0)

Expand Down
14 changes: 0 additions & 14 deletions Lib/test/test_zipfile/_path/_itertools.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,20 +29,6 @@ def __next__(self):
return result


# from more_itertools v8.13.0
def always_iterable(obj, base_type=(str, bytes)):
if obj is None:
return iter(())

if (base_type is not None) and isinstance(obj, base_type):
return iter((obj,))

try:
return iter(obj)
except TypeError:
return iter((obj,))


# from more_itertools v9.0.0
def consume(iterator, n=None):
"""Advance *iterable* by *n* steps. If *n* is ``None``, consume it
Expand Down
2 changes: 1 addition & 1 deletion Lib/test/test_zipfile/_path/test_path.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@
import zipfile._path

from test.support.os_helper import temp_dir, FakePath
from test.support.parameterize import parameterize, Invoked

from ._functools import compose
from ._itertools import Counter

from ._test_params import parameterize, Invoked
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cosmetic detail: one more line should be removed here (to leave vertical space of 2 lines, rather than of 3 lines).



class jaraco:
Expand Down
1 change: 1 addition & 0 deletions Misc/NEWS.d/next/Library/2024-07-22-23-18-57.gh-issue-121237.WkbIpy.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Accept "%:z" in strptime
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When releasing, changes will be added to the changelog. You should describe your changes in detail but keep it concise. For example:

Added %:Z used for XXX to strptime.

(I'm sorry, I don't know what %:z does).

Copy link
Contributor

@zuo zuo Oct 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd propose something along the lines:

Suggested change
Accept "%:z" in strptime
:meth:`~datetime.datetime.strptime` now supports the ``%:z`` format code.

Loading