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

timezone allows no offset from range (23:59, 24:00) #81823

Closed
joernheissler mannequin opened this issue Jul 21, 2019 · 6 comments
Closed

timezone allows no offset from range (23:59, 24:00) #81823

joernheissler mannequin opened this issue Jul 21, 2019 · 6 comments
Labels
3.7 (EOL) end of life 3.8 only security fixes 3.9 only security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@joernheissler
Copy link
Mannequin

joernheissler mannequin commented Jul 21, 2019

BPO 37642
Nosy @abalkin, @pganssle, @joernheissler, @tirkarthi
PRs
  • bpo-37642: Update max and min offset in datetime module #14878
  • [3.7] bpo-37642: Update max and min offset in datetime module (GH-14878) #15226
  • [3.8] bpo-37642: Update max and min offset in datetime module (GH-14878) #15227
  • 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-08-15.19:11:42.809>
    created_at = <Date 2019-07-21.11:41:24.145>
    labels = ['3.7', '3.8', 'type-bug', 'library', '3.9']
    title = 'timezone allows no offset from range (23:59, 24:00)'
    updated_at = <Date 2019-08-15.19:11:42.808>
    user = 'https://github.com/joernheissler'

    bugs.python.org fields:

    activity = <Date 2019-08-15.19:11:42.808>
    actor = 'p-ganssle'
    assignee = 'none'
    closed = True
    closed_date = <Date 2019-08-15.19:11:42.809>
    closer = 'p-ganssle'
    components = ['Library (Lib)']
    creation = <Date 2019-07-21.11:41:24.145>
    creator = 'joernheissler'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 37642
    keywords = ['patch']
    message_count = 6.0
    messages = ['348237', '348242', '348298', '349290', '349816', '349817']
    nosy_count = 4.0
    nosy_names = ['belopolsky', 'p-ganssle', 'joernheissler', 'xtreak']
    pr_nums = ['14878', '15226', '15227']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue37642'
    versions = ['Python 3.7', 'Python 3.8', 'Python 3.9']

    @joernheissler
    Copy link
    Mannequin Author

    joernheissler mannequin commented Jul 21, 2019

    https://bugs.python.org/issue5288 changed datetime.timezone to accept sub-minute offsets.

    The C implementation allows offsets from range (23:59, 24:00) while the python implementation does not:

    # C
    >>> timezone(timedelta(seconds=86399))
    datetime.timezone(datetime.timedelta(seconds=86399))
    
    # Python
    >>> timezone(timedelta(seconds=86399))
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "cpython/Lib/datetime.py", line 2194, in __new__
        raise ValueError("offset must be a timedelta "
    ValueError: offset must be a timedelta strictly between -timedelta(hours=24) and timedelta(hours=24).

    This is because _maxoffset is defined as timedelta(hours=23, minutes=59)

    Second issue: The (undocumented) "min" and "max" attributes (both C and python) show 23:59
    even though the C implementation can get closer to 24:00.
    Should this be changed to timezone(timedelta(seconds=86399, microseconds=999999))?

    (Same applies to the minimums)

    @joernheissler joernheissler mannequin added 3.7 (EOL) end of life 3.8 only security fixes 3.9 only security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Jul 21, 2019
    @pganssle
    Copy link
    Member

    I agree that the C and Python behavior should be the same, and both of them should allow all offsets less than 24 hours. I'm actually quite surprised we don't have a test for this in datetimetester.py.

    @pganssle
    Copy link
    Member

    I have been thinking about this more and I think the two issues (min and max vs. the incompatibility between C and Python) should be considered two separate issues.

    My reasoning is that a change of the value of timezone.min and timezone.max is not something I'd be comfortable backporting to 3.7, because it has the potential to cause failures in some test suites (for example, datetime.now(tz=datetime.timezone.max).isoformat() will start returning a string that does not conform to ISO 8601, which has no support for sub-minute offsets). However, differences between the C and Python implementations are a violation of at least the spirit of PEP-399 and I think it should be backported to 3.7.

    @pganssle
    Copy link
    Member

    pganssle commented Aug 9, 2019

    New changeset 92c7e30 by Paul Ganssle (Ngalim Siregar) in branch 'master':
    bpo-37642: Update acceptable offsets in timezone (GH-14878)
    92c7e30

    @pganssle
    Copy link
    Member

    New changeset 27b38b9 by Paul Ganssle in branch '3.8':
    bpo-37642: Update acceptable offsets in timezone (GH-14878) (bpo-15227)
    27b38b9

    @pganssle
    Copy link
    Member

    New changeset ed44b84 by Paul Ganssle in branch '3.7':
    bpo-37642: Update acceptable offsets in timezone (GH-14878) (bpo-15226)
    ed44b84

    @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 3.9 only security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    1 participant