classification
Title: Inconsistency in datetime.utcfromtimestamp(Decimal)
Type: behavior Stage: patch review
Components: Library (Lib) Versions: Python 3.8
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: serhiy.storchaka Nosy List: SilentGhost, Steven Davidson, anglister, belopolsky, lemburg, p-ganssle, remi.lapeyre, serhiy.storchaka, skrah, vstinner
Priority: low Keywords: patch

Created on 2015-03-08 10:35 by serhiy.storchaka, last changed 2019-02-27 17:14 by remi.lapeyre.

Files
File name Uploaded Description Edit
issue23607_python.diff SilentGhost, 2015-03-08 15:50 review
datetime_fromtimestamp_decimal.patch serhiy.storchaka, 2015-03-30 10:04 review
Messages (11)
msg237527 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-03-08 10:35
First, datetime.utcfromtimestamp() drops fractional part of Decimal argument:

>>> import datetime
>>> from decimal import Decimal as D
>>> datetime.datetime.utcfromtimestamp(1425808327.307651)
datetime.datetime(2015, 3, 8, 9, 52, 7, 307651)
>>> datetime.datetime.utcfromtimestamp(D(1425808327.307651))
datetime.datetime(2015, 3, 8, 9, 52, 7)

Second, it works only with C implementation. With Python implementation Decimals are not supported at all.

>>> del sys.modules['datetime']
>>> sys.modules['_datetime'] = None
>>> import datetime
>>> datetime.datetime.utcfromtimestamp(D(1425808327.307651))
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib/python3.4/datetime.py", line 1374, in utcfromtimestamp
    t, frac = divmod(t, 1.0)
TypeError: unsupported operand type(s) for divmod(): 'decimal.Decimal' and 'float'

In 2.7 Decimals are accepted and are not truncated.

>>> import datetime
>>> from decimal import Decimal as D
>>> datetime.datetime.utcfromtimestamp(D(1425808327.307651))
datetime.datetime(2015, 3, 8, 9, 52, 7, 307651)

So this can be considered as a regression.
msg237541 - (view) Author: SilentGhost (SilentGhost) * (Python triager) Date: 2015-03-08 15:50
Here is the python-only fix that eliminates TypeError and produces correct number of milliseconds. The problem with C implementation lies in _PyTime_ObjectToDenominator function where there is an explicit check for a float. This check could be relaxed to include decimals as the rest of the code seem to work fine with decimal. I'm not entirely sure how that could be done though.
msg237801 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-03-10 19:24
The fix for Python implementation LGTM, but a fix for C implementation is needed too.
msg239593 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-03-30 10:04
Here is a patch that adds Decimal support for datetime.fromtimestamp() and datetime.utcfromtimestamp(). As side effect Decimal timestamps now are supported in few other places, e.g. in os.utime().
msg239595 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2015-03-30 11:36
I reviewed datetime_fromtimestamp_decimal.patch.

> As side effect Decimal timestamps now are supported in few other places, e.g. in os.utime().

It would be more consistent to support decimal.Decimal nowhere or "everywhere". IMO the new _PyTime_FromSecondsObject() (very close to _PyTime_ObjectToDenominator, but using time_t) should also be patched.

Please add some tests for decimal.Decimal in test_time directly. Usually, I try to test rounding and overflow. Testing for overflow is not always possible because it may depend on the platform.

--

See also the PEP 410: "PEP 410 - Use decimal.Decimal type for timestamps". The PEP was rejected. The compromise was a new timestamp format for os.utime(): a number of nanoseconds since the UNIX epoch (1970-01-01). I partially implemented a private API for this PEP in the issue #22117: the new _PyTime_t is a number of timestamp using an arbitrary resolution (currently, it's a number of nanoseconds using a 64-bit signed integer type).
msg239597 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2015-03-30 11:55
I just looked at this very briefly: Is the patch context insensitive?

IOW, do things still work if you change the thread-local context:

from decimal import *
c = getcontext()
c.prec = 1
c.Emax = 1
c.Emin = -1
msg239608 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-03-30 13:48
The problem is that the Decimal *was* supported in 2.7. So this issue can be considered not as adding new feature, but as fixing a regression. But changes are too large for just bugfix.

> It would be more consistent to support decimal.Decimal nowhere or "everywhere". IMO the new _PyTime_FromSecondsObject() (very close to _PyTime_ObjectToDenominator, but using time_t) should also be patched.

Will add Decimal support in all functions in Python/pytime.c that support floats.

> Please add some tests for decimal.Decimal in test_time directly. Usually, I try to test rounding and overflow. Testing for overflow is not always possible because it may depend on the platform.

Will do.

> Is the patch context insensitive?

No, the patch is context sensitive. I think the end user is responsible to set an appropriate context if it want to create a datetime from Decimal timestamp. At least until we add functions that return Decimal.
msg239613 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2015-03-30 14:42
I think we should try to avoid depending on global state in
the stdlib, at least in new code.  Also, if something is not
really a decimal computation, Decimal itself tries to ignore
the context (like Decimal.__repr__).

At least I would expect this datetime function to be context
independent.
msg336769 - (view) Author: Paul Ganssle (p-ganssle) * (Python committer) Date: 2019-02-27 16:53
I'm not sure if either of these patches got merged, but at some point this was fixed:

    Python 3.7.2 (default, Feb  9 2019, 13:18:43) 
    [GCC 8.2.1 20181127] on linux
    Type "help", "copyright", "credits" or "license" for more information.
    >>> from datetime import datetime
    >>> from decimal import Decimal
    >>> datetime.utcfromtimestamp(Decimal(123456.0))
    datetime.datetime(1970, 1, 2, 10, 17, 36)


I recommend that someone add some regression tests to this, then we can close the issue.

I'm also going to mark this as "easy" since adding tests for this particular example should be pretty simple.
msg336771 - (view) Author: Paul Ganssle (p-ganssle) * (Python committer) Date: 2019-02-27 17:10
Oh actually that's my mistake. I can't reproduce the failure in the constructor in the Python version of the module, and also it seems to be fixed in the pure Python version as of at least 3.6:

Python 3.6.7 (default, Oct 25 2018, 16:11:17) 
[GCC 8.2.1 20180831] on linux
>>> import sys
>>> sys.modules['_datetime'] = None
>>> from decimal import Decimal as D
>>> from datetime import datetime
>>> datetime.utcfromtimestamp(D(123456.12345))
datetime.datetime(1970, 1, 2, 10, 17, 36, 123450)

But the truncation behavior is still present in the C version as of Python 3.8.0a1+:

Python 3.8.0a1+ (heads/master:3766f18, Feb 11 2019, 12:52:31) 
[GCC 8.2.1 20181127] on linux
>>> from datetime import datetime
>>> from decimal import Decimal as D
>>> datetime.utcfromtimestamp(D(123456.12345))
datetime.datetime(1970, 1, 2, 10, 17, 36)


I still think we need a test for the constructor behavior, but I'm going to remove "easy", since we still need to fix truncation in the C version.
msg336772 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2019-02-27 17:11
No, the bug is not fixed, and this is not easy issue. You should use non-integer Decimals to reproduce it. In 3.8 this emits a deprecation warning:

>>> import datetime
>>> from decimal import Decimal as D
>>> datetime.datetime.utcfromtimestamp(D(1425808327.307651))
<stdin>:1: DeprecationWarning: an integer is required (got type decimal.Decimal).  Implicit conversion to integers using __int__ is deprecated, and may be removed in a future version of Python.
datetime.datetime(2015, 3, 8, 9, 52, 7)
History
Date User Action Args
2019-02-27 17:14:23remi.lapeyresetnosy: + remi.lapeyre
2019-02-27 17:11:22serhiy.storchakasetmessages: + msg336772
2019-02-27 17:10:29p-gansslesetkeywords: - easy

messages: + msg336771
2019-02-27 16:53:12p-gansslesetkeywords: + easy

messages: + msg336769
2019-02-08 13:20:50Steven Davidsonsetnosy: + Steven Davidson
2018-07-05 15:09:00p-gansslesetnosy: + p-ganssle
2018-05-12 13:00:36serhiy.storchakasetpriority: normal -> low
assignee: serhiy.storchaka
versions: + Python 3.8, - Python 3.4, Python 3.5
2018-05-09 05:27:36anglistersetnosy: + anglister
2018-05-05 13:32:26serhiy.storchakalinkissue33296 superseder
2015-03-30 14:42:46skrahsetmessages: + msg239613
2015-03-30 13:48:12serhiy.storchakasetmessages: + msg239608
2015-03-30 11:55:17skrahsetnosy: + skrah
messages: + msg239597
2015-03-30 11:36:57vstinnersetmessages: + msg239595
2015-03-30 10:04:40serhiy.storchakasetfiles: + datetime_fromtimestamp_decimal.patch

nosy: + vstinner
messages: + msg239593

stage: needs patch -> patch review
2015-03-10 19:24:00serhiy.storchakasetmessages: + msg237801
stage: patch review -> needs patch
2015-03-10 19:11:47serhiy.storchakasetstage: needs patch -> patch review
2015-03-08 15:50:35SilentGhostsetfiles: + issue23607_python.diff

nosy: + SilentGhost
messages: + msg237541

keywords: + patch
stage: needs patch
2015-03-08 10:35:01serhiy.storchakacreate