classification
Title: Different behavior between datetime.py and its C accelerator
Type: behavior Stage: resolved
Components: Extension Modules, Library (Lib) Versions: Python 3.8, Python 3.7, Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: belopolsky Nosy List: belopolsky, tim.peters
Priority: normal Keywords: patch

Created on 2018-06-09 00:02 by belopolsky, last changed 2018-06-10 22:06 by belopolsky. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 7578 merged belopolsky, 2018-06-10 03:17
PR 7600 merged miss-islington, 2018-06-10 21:04
PR 7601 merged miss-islington, 2018-06-10 21:05
Messages (9)
msg319121 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2018-06-09 00:08
Consider the following code:

$ cat bug.py
import sys
if len(sys.argv) > 1:
    sys.modules['_datetime'] = None
from datetime import tzinfo, datetime, timezone

class TZ(tzinfo):
    def utcoffset(self, t):
        pass

print(datetime(2000,1,1,tzinfo=TZ()).astimezone(timezone.utc))

When running with no arguments (with C acceleration), I get

$ ./python.exe bug.py
2000-01-01 00:00:00+00:00

but the pure python code produces an error

$ ./python.exe bug.py pure
Traceback (most recent call last):
  File "bug.py", line 10, in <module>
    print(datetime(2000,1,1,tzinfo=TZ()).astimezone(timezone.utc))
  File ".../Lib/datetime.py", line 1783, in astimezone
    raise ValueError("astimezone() requires an aware datetime")
ValueError: astimezone() requires an aware datetime

Note that some kind of error is expected because TZ.utcoffset() returns None instead of a timedelta, but the error message produced by pure python code is confusing.
msg319122 - (view) Author: Tim Peters (tim.peters) * (Python committer) Date: 2018-06-09 00:17
The message isn't confusing - the definition of "aware" is confusing ;-)

"""
A datetime object d is aware if d.tzinfo is not None and d.tzinfo.utcoffset(d) does not return None. If d.tzinfo is None, or if d.tzinfo is not None but d.tzinfo.utcoffset(d) returns None, d is naive.
"""
msg319131 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2018-06-09 03:00
So you are suggesting that my datetime(2000,1,1,tzinfo=TZ()) should behave as a naive instance, right?  Well, this would be a third behavior different from both current C and Python implementations:

>>> print(datetime(2000,1,1).astimezone(timezone.utc))
2000-01-01 05:00:00+00:00

(I am in US/Eastern timezone.)
msg319134 - (view) Author: Tim Peters (tim.peters) * (Python committer) Date: 2018-06-09 03:31
I copy/pasted the definitions of "aware" and "naive" from the docs.  Your TZ's .utcoffset() returns None, so, yes, any datetime using an instance of that for its tzinfo is naive.

In

print(datetime(2000,1,1).astimezone(timezone.utc))

the docs for astimezone say, in part,

"""
If self is naive (self.tzinfo is None), it is presumed to represent time in the system timezone.
"""

So it converted your naive time (viewed as being in your system - EDT - time zone) to UTC.

That appears to be using a different definition of "naive" (looking only at whether self.tzinfo is None).

The original datetime.py certainly didn't do that ...

"""
def astimezone(self, tz):
    if not isinstance(tz, tzinfo):
        raise TypeError("tz argument must be an instance of tzinfo")
    mytz = self.tzinfo
    if mytz is None:
        raise ValueError("astimezone() requires an aware datetime")

    if tz is mytz:
        return self

    # Convert self to UTC, and attach the new time zone object.
    myoffset = self.utcoffset()
    if myoffset is None:
        raise ValueError("astimezone() requires an aware datetime")
"""

So it originally used the definition I quoted first.  The "sometimes pretend it's local time anyway" twist appeared to get added here:

https://github.com/python/cpython/commit/fdc860f3106b59ec908e0b605e51a1607ea2ff4b
msg319195 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2018-06-10 03:45
Tim, given that I've updated the documentation, should we treat this as a bug fix or a feature?  Note that the type check is definitely a bug-fix (if not a security issue), but I clearly had a wrong definition of "naive" in mind when I was modifying astimezone to support naive instances.

In any case, it looks like a news entry is in order.
msg319196 - (view) Author: Tim Peters (tim.peters) * (Python committer) Date: 2018-06-10 03:51
I'd call it a bug fix, but I'm really not anal about what people call things ;-)
msg319245 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2018-06-10 21:03
New changeset 877b23202b7e7d4f57b58504fd0eb886e8c0b377 by Alexander Belopolsky in branch 'master':
bpo-33812: Corrected astimezone for naive datetimes. (GH-7578)
https://github.com/python/cpython/commit/877b23202b7e7d4f57b58504fd0eb886e8c0b377
msg319253 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2018-06-10 22:02
New changeset 037e9125527d4a55af566f161c96a61b3c3fd998 by Alexander Belopolsky (Miss Islington (bot)) in branch '3.7':
bpo-33812: Corrected astimezone for naive datetimes. (GH-7578) (GH-7600)
https://github.com/python/cpython/commit/037e9125527d4a55af566f161c96a61b3c3fd998
msg319254 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2018-06-10 22:03
New changeset 1d4089b5d208ae6f0bd256304fd77f04c0b4fd41 by Alexander Belopolsky (Miss Islington (bot)) in branch '3.6':
bpo-33812: Corrected astimezone for naive datetimes. (GH-7578) (GH-7601)
https://github.com/python/cpython/commit/1d4089b5d208ae6f0bd256304fd77f04c0b4fd41
History
Date User Action Args
2018-06-10 22:06:00belopolskysetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2018-06-10 22:03:33belopolskysetmessages: + msg319254
2018-06-10 22:02:26belopolskysetmessages: + msg319253
2018-06-10 21:05:11miss-islingtonsetpull_requests: + pull_request7223
2018-06-10 21:04:15miss-islingtonsetstage: commit review -> patch review
pull_requests: + pull_request7222
2018-06-10 21:03:01belopolskysetmessages: + msg319245
2018-06-10 18:14:12belopolskysetstage: patch review -> commit review
versions: + Python 3.6
2018-06-10 03:51:20tim.peterssetmessages: + msg319196
2018-06-10 03:45:15belopolskysetmessages: + msg319195
2018-06-10 03:17:24belopolskysetkeywords: + patch
stage: needs patch -> patch review
pull_requests: + pull_request7202
2018-06-09 03:31:22tim.peterssetmessages: + msg319134
2018-06-09 03:02:00belopolskysetassignee: belopolsky
stage: needs patch
type: behavior
components: + Extension Modules, Library (Lib)
versions: + Python 3.7, Python 3.8
2018-06-09 03:00:52belopolskysetmessages: + msg319131
2018-06-09 00:17:42tim.peterssetnosy: + tim.peters
messages: + msg319122
2018-06-09 00:08:08belopolskysetmessages: + msg319121
title: Different behavior betwee -> Different behavior between datetime.py and its C accelerator
2018-06-09 00:02:26belopolskycreate