classification
Title: Datetime “fromtimestamp()” ignores inheritance if timezone is not None
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.8
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Delgan, belopolsky, lukasz.langa, nixphix, p-ganssle, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2018-11-30 17:25 by Delgan, last changed 2019-02-14 16:30 by p-ganssle. This issue is now closed.

Files
File name Uploaded Description Edit
test.py Delgan, 2018-11-30 17:25
Pull Requests
URL Status Linked Edit
PR 10902 merged p-ganssle, 2018-12-04 18:30
PR 11790 merged p-ganssle, 2019-02-08 15:24
PR 11790 merged p-ganssle, 2019-02-08 15:24
Messages (10)
msg330811 - (view) Author: Adrien (Delgan) * Date: 2018-11-30 17:25
Hello.

I created a class inheriting from "datetime.datetime".

While creating a new instance using the classmethod "fromtimestamp" it seems to work, except if I provide a timezone object. In such case, the returned object is of base type datetime.

This looks like a bug, isn't it? If not, I guess this should be mentioned somewhere in the documentation. Tested on Python 3.6 and 3.7, my apologies if this has been fixed in 3.8.

NB: I first opened a question on SO -> https://stackoverflow.com/questions/53561996/datetime-fromtimestamp-ignores-inheritance-if-timezone-is-not-none
msg330815 - (view) Author: Adrien (Delgan) * Date: 2018-11-30 17:34
Actually, this also occurs while using "astimezone()" on a custom child class (I suppose this method is used by "fromtimestamp()").
msg330892 - (view) Author: Prabakaran Kumaresshan (nixphix) * Date: 2018-12-02 17:23
It's a side effect of a date arithmetic operation performed while setting timezone.

while passing tz info to datetime invokes tz.fromutc() -> https://github.com/python/cpython/blob/3df85404d4bf420db3362eeae1345f2cad948a71/Lib/datetime.py#L1593

followed by a datetime arithmetic here -> https://github.com/python/cpython/blob/3df85404d4bf420db3362eeae1345f2cad948a71/Lib/datetime.py#L2188

which eventually leads to building a new datetime object here ->
https://github.com/python/cpython/blob/3df85404d4bf420db3362eeae1345f2cad948a71/Lib/datetime.py#L1997-L2000

I'm not sure if its an expected behaviour
msg331056 - (view) Author: Paul Ganssle (p-ganssle) * (Python triager) Date: 2018-12-04 15:43
This is another manifestation of issue 32417.

The biggest complication, to me, is that adding a `timedelta` to a datetime always returns a `datetime.datetime` rather than the subclass that it was added to.

I *think* most if not all people would consider this a bug, and we can probably fix it. That should cascade down to `fromutc` and then to `astimezone`.
msg331058 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-12-04 16:04
This is not easy problem, ant it doesn't have right solution. Different decisions were made for the result type of alternate constructors and operators for different classes.
 
The frombytes() method of an int subclass returns an int. As well arithmetic operations with int subclasses return an int.

The fromhex() method of a bytes subclass returns an instance of this subclass. But arithmetic operations with bytes subclasses return a bytes object.

The fromkeys() method of a dict subclass returns an instance of this subclass. The copy() method of a dict subclass returns a dict. The copy() method of a dict subclass returns a dict. But defaultdict, OrdertedDict and Counter redefine it: copy() methods of their subclasses return the object of the same type.
msg331065 - (view) Author: Paul Ganssle (p-ganssle) * (Python triager) Date: 2018-12-04 17:13
> This is not easy problem, ant it doesn't have right solution. Different decisions were made for the result type of alternate constructors and operators for different classes.

It's certainly true that it occasionally makes sense to do something like this, but this one is particularly egregious. Because of an implementation detail of `fromtimestamp` (and `now`, and a few others), which is actually relying on an implementation detail of `fromutc`, which is actually relying on what *may* have been a choice in the `__add__` method of timedelta, you get a *different class* in the alternate constructor of a subclass depending on the *argument* to the alternate constructor. This is pretty solidly a bug.

I think the things we need to take into account are:

1. What do we consider the contract of the relevant functions involved
2. What do people expect the code to do?
3. Is it likely that anyone is *relying* on the existing behavior.


The most fundamental problem, timedelta addition, is also the closest call, so I think we should *start* with the analysis there.

For #1, the contract is either "x + timedelta returns a datetime if x is a datetime subclass" or "x + timedelta returns a datetime or datetime subclass" - both of these are consistent with the current behavior, and as long as "datetime subclass isa datetime", I don't see that there would be anything fundamentally backwards-incompatible about changing what is actually returned.

For #2, I think people almost universally consider it a bug or at the very least counter-intuitive that `DatetimeSubclass + timedelta` returns a datetime and not a DatetimeSubclass. There are many instances of people who create datetime subclasses like arrow and pendulum (for just two OSS examples) - all of them end up with fairly complicated implementations where they have to work around all the places where the underlying implementation has hard-coded datetime. Some of these have been fixed already, but it's a notorious game of whack-a-mole. I've never heard of a situation where someone *wants* the other behavior.

For #3, it is feasible that there are people who are accidentally relying on this behavior, but it would only be in pretty unpythonic code (like assert type(dt) == datetime), or when using broken datetime subclasses. The only situation where I can think of where this behavior might be desirable is if you have a thin datetime wrapper that only needs to be the wrapper class at the point of input, and afterwards you don't care what class it is (and as such you'd want it to be datetime, so as to take advantage of the fast paths in C code). That is far from the common case, and it's a "nice to have" - if it doesn't happen you'll get slower code, not broken code, and you can fix it the same way everyone else fixes their broken subclasses by overriding __add__ and __radd__.

I think there is a pretty compelling case for switching timedelta + datetimesubclass over to returning `datetimesubclass`.
msg331067 - (view) Author: Paul Ganssle (p-ganssle) * (Python triager) Date: 2018-12-04 17:22
Another thing to note: I'm pretty sure this was a mistake in the first place. There are many examples of places where the datetime module was just not designed with inheritance in mind, for example:

- issue 32404 / issue 32403 - fromtimestamp not calling __new__
- issue 31222 / 20371  - C implementation of .replace

I think there are many other unreported issues that all stem from similar inconsistencies that we've been slowly shoring up.

One problem is that it's very inconsistent, which makes datetime not particularly friendly to subclass, but to the extent that that's changing, we've been getting *more* friendly to subclasses.
msg334838 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2019-02-04 19:42
New changeset 89427cd0feae25bbc8693abdccfa6a8c81a2689c by Alexander Belopolsky (Paul Ganssle) in branch 'master':
bpo-32417: Make timedelta arithmetic respect subclasses (#10902)
https://github.com/python/cpython/commit/89427cd0feae25bbc8693abdccfa6a8c81a2689c
msg335092 - (view) Author: Łukasz Langa (lukasz.langa) * (Python committer) Date: 2019-02-08 16:02
New changeset d9503c307a8b6a7b73f6344183602ffb014d3356 by Łukasz Langa (Paul Ganssle) in branch 'master':
Add What's New entry for date subclass behavior (#11790)
https://github.com/python/cpython/commit/d9503c307a8b6a7b73f6344183602ffb014d3356
msg335135 - (view) Author: Paul Ganssle (p-ganssle) * (Python triager) Date: 2019-02-09 17:50
This issue and bpo-32417 can be closed now, as they are fixed on master.
History
Date User Action Args
2019-02-14 16:30:18p-gansslesetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2019-02-09 17:50:06p-gansslesetmessages: + msg335135
versions: + Python 3.8, - Python 3.6, Python 3.7
2019-02-08 16:02:17lukasz.langasetnosy: + lukasz.langa
messages: + msg335092
2019-02-08 15:24:54p-gansslesetpull_requests: + pull_request11795
2019-02-08 15:24:43p-gansslesetpull_requests: + pull_request11794
2019-02-04 19:42:17belopolskysetnosy: + belopolsky
messages: + msg334838
2018-12-04 18:30:47p-gansslesetkeywords: + patch
stage: patch review
pull_requests: + pull_request10143
2018-12-04 17:22:35p-gansslesetmessages: + msg331067
2018-12-04 17:13:33p-gansslesetmessages: + msg331065
2018-12-04 16:04:28serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg331058
2018-12-04 15:43:15p-gansslesetnosy: + p-ganssle
messages: + msg331056
2018-12-02 17:23:31nixphixsetnosy: + nixphix
messages: + msg330892
2018-11-30 17:34:20Delgansetmessages: + msg330815
2018-11-30 17:25:41Delgancreate