time.sleep() should support objects with __float__

[jdemeyer]

BPO	35707
Nosy	@ronaldoussoren, @ncoghlan, @abalkin, @vstinner, @serhiy-storchaka, @jdemeyer, @MojoVampire, @pganssle, @remilapeyre
PRs	bpo-26669: Reject float infinity in time functions #11507 bpo-26669: Reject float infinity in time functions #11507 gh-79888: support __index__ and __float__ in time functions #11636 gh-79888: support __index__ and __float__ in time functions #11636 gh-79888: support __index__ and __float__ in time functions #11636

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2019-01-10.15:58:39.782>
labels = ['interpreter-core', '3.10']
title = 'time.sleep() should support objects with __float__'
updated_at = <Date 2020-05-23.00:14:27.187>
user = 'https://github.com/jdemeyer'

bugs.python.org fields:

activity = <Date 2020-05-23.00:14:27.187>
actor = 'cheryl.sabella'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Interpreter Core']
creation = <Date 2019-01-10.15:58:39.782>
creator = 'jdemeyer'
dependencies = []
files = []
hgrepos = []
issue_num = 35707
keywords = ['patch', 'patch']
message_count = 29.0
messages = ['333391', '333395', '333396', '333397', '333401', '333404', '333416', '333547', '333550', '333554', '333555', '333961', '333963', '333968', '333969', '334120', '334121', '334122', '334125', '334128', '334129', '334131', '334136', '334480', '334486', '334509', '339123', '349004', '349560']
nosy_count = 10.0
nosy_names = ['ronaldoussoren', 'ncoghlan', 'belopolsky', 'vstinner', 'serhiy.storchaka', 'jdemeyer', 'josh.r', 'p-ganssle', 'remi.lapeyre', 'AVINASH MISHRA']
pr_nums = ['11507', '11507', '11636', '11636', '11636']
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = None
url = 'https://bugs.python.org/issue35707'
versions = ['Python 3.10']

[jdemeyer]

This used to work correctly in Python 2:

class Half(object):
    def __float__(self):
        return 0.5
import time
time.sleep(Half())

With Python 3.6, one gets instead

Traceback (most recent call last):
  File "test.py", line 6, in <module>
    time.sleep(Half())
TypeError: an integer is required (got type Half)

[AVINASHMISHRA]

hey i am a total newbie to open source contribution.
can you help me understand this issue and can i help solve this issue?

[remilapeyre]

time.sleep() is probably not the only function to have such a bug.

Maybe __int__() should default to:

    def __int__(self):
        return int(self.__float__())

when __float__ is defined and not __int__.

Nick Coghlan suggested something similar for __int__ and __index__.

[remilapeyre]

See bpo-33039 for the proposed change to __int__.

[vstinner]

The problem comes from the private C function _PyTime_FromObject() of Python/pytime.c. This function must use the proper conversion to minimize the precision loss. Lib/test/test_time.py contains a lot of tests on conversions from different types and ensure that values are rounded correctly. See also my PEP-564 "Add new time functions with nanosecond resolution".

The correct code works for float and int (and maybe decimal.Decimal, I don't recall!), but it seems like it doesn't support types with __float__(). You have to explicitly cast such objects using float(value).

PyNumber_Float() can be used to convert arbitrary object to a float, but I'm not sure in which order the conversion should be tried to avoid/reduce precision loss during the conversion.

Example:

>>> x=2**53+1; x - int(float(x))
1

If we convert 'x' (int) to float, we introduce an error of 1.

[vstinner]

PR 11507 is not directly related to this issue, see bpo-26669. But I wrote this PR when trying to fix this issue :-)

[jdemeyer]

I'm not sure in which order the conversion should be tried to avoid/reduce precision loss during the conversion.

I would suggest the order

1. __index__ to ensure exact conversion of exact integers
2. __float__ to ensure correct conversion of floating-point numbers
3. __int__

[ncoghlan]

Deriving __int__ from __float__ wouldn't be the right answer, as that can easily lead to unwanted OverflowError exceptions and other issues.

However, Jeroen's suggested order of checking __index__ then __float__ then __int__ in _PyTime_FromObject makes sense to me, as that addresses Victor's desire to use the most precise conversion available.

[serhiy-storchaka]

This can cause a loss of precision for Decimal.

If we want to support other numerical types with loss in double rounding, the most reliable way is to represent them as fractions (x.as_integer_ratio() or (x.numerator, x.denominator)) and use precise integer arithmetic.

[jdemeyer]

the most reliable way is to represent them as fractions (x.as_integer_ratio() or (x.numerator, x.denominator))

I don't think that we can rely on non-dunder names like that. They are not reserved names, so classes can give them any semantics that they like. This is not just hypothetical: SageMath for example uses numerator() and denominator() methods, not properties.

If you really want to go through with this, probably a special method like __as_integer_ratio__ should be defined.

Anyway, I personally consider the double rounding for time.sleep() a non-issue. We are not trying to write a precise math library here, nobody will complain about sleeping a femtosecond too long.

[jdemeyer]

The correct code works for float and int (and maybe decimal.Decimal, I don't recall!)

Not for Decimal! In fact sleep(Decimal("0.99")) is interpreted as sleep(0) because __int__ is used to convert.

[jdemeyer]

My proposal vastly improves the situation for Decimal. I will write a PR for this and I hope that it won't be rejected just because it's not perfect.

[vstinner]

Not for Decimal! In fact sleep(Decimal("0.99")) is interpreted as sleep(0) because __int__ is used to convert.

Oh oh. I didn't know that. It should be fixed.