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

Created on 2018-11-30 17:25 by Delgan, last changed 2018-12-04 18:30 by p-ganssle.

Files
File name Uploaded Description Edit
test.py Delgan, 2018-11-30 17:25
Pull Requests
URL Status Linked Edit
PR 10902 open p-ganssle, 2018-12-04 18:30
Messages (7)
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) * 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) * 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) * 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.
History
Date User Action Args
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