Issue 35364: Datetime “fromtimestamp()” ignores inheritance if timezone is not None (original) (raw)

Created on 2018-11-30 17:25 by Delgan, last changed 2022-04-11 14:59 by admin. This issue is now closed.

Messages (10)

msg330811 - (view)

Author: Delgan (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: Delgan (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 committer)

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 committer)

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 committer)

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:

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 committer)

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

2022-04-11 14:59:08

admin

set

github: 79545

2019-02-14 16:30:18

p-ganssle

set

status: open -> closed
resolution: fixed
stage: patch review -> resolved

2019-02-09 17:50:06

p-ganssle

set

messages: +
versions: + Python 3.8, - Python 3.6, Python 3.7

2019-02-08 16:02:17

lukasz.langa

set

nosy: + lukasz.langa
messages: +

2019-02-08 15:24:54

p-ganssle

set

pull_requests: + <pull%5Frequest11795>

2019-02-08 15:24:43

p-ganssle

set

pull_requests: + <pull%5Frequest11794>

2019-02-04 19:42:17

belopolsky

set

nosy: + belopolsky
messages: +

2018-12-04 18:30:47

p-ganssle

set

keywords: + patch
stage: patch review
pull_requests: + <pull%5Frequest10143>

2018-12-04 17:22:35

p-ganssle

set

messages: +

2018-12-04 17:13:33

p-ganssle

set

messages: +

2018-12-04 16:04:28

serhiy.storchaka

set

nosy: + serhiy.storchaka
messages: +

2018-12-04 15:43:15

p-ganssle

set

nosy: + p-ganssle
messages: +

2018-12-02 17:23:31

nixphix

set

nosy: + nixphix
messages: +

2018-11-30 17:34:20

Delgan

set

messages: +

2018-11-30 17:25:41

Delgan

create