[issue35707] time.sleep() should support objects with __float__

Change by Cheryl Sabella : -- nosy: +belopolsky, p-ganssle versions: +Python 3.10 -Python 3.6, Python 3.7, Python 3.8 ___ Python tracker ___

[issue35707] time.sleep() should support objects with __float__

STINNER Victor added the comment: See also bpo-20861. -- ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue35707] time.sleep() should support objects with __float__

Jeroen Demeyer added the comment: > 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)) See

[issue35707] time.sleep() should support objects with __float__

Jeroen Demeyer added the comment: Is anybody willing to review PR 11636? -- ___ Python tracker ___ ___ Python-bugs-list mailing

[issue35707] time.sleep() should support objects with __float__

Jeroen Demeyer added the comment: > You've got a reference leak in your __index__ based paths. Thanks for pointing that out. I fixed that now. -- ___ Python tracker ___

[issue35707] time.sleep() should support objects with __float__

Jeroen Demeyer added the comment: > I'm also mildly concerned by how duplicative the code becomes post-patch. I know, that's why I added that comment on GitHub. > perhaps just implement _PyTime_ObjectToTime_t as a wrapper for > _PyTime_ObjectToDenominator Sure, but will that increase the

[issue35707] time.sleep() should support objects with __float__

Josh Rosenberg added the comment: You've got a reference leak in your __index__ based paths. PyNumber_Index is returning a new reference (either to the existing obj, or a new one, if the existing obj isn't already an int). You never release this reference. Simplest fix is to make intobj top

[issue35707] time.sleep() should support objects with __float__

Change by Jeroen Demeyer : -- components: +Interpreter Core ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue35707] time.sleep() should support objects with __float__

Change by Jeroen Demeyer : -- pull_requests: +11407, 11408, 11409 ___ Python tracker ___ ___ Python-bugs-list mailing list

[issue35707] time.sleep() should support objects with __float__

Change by Jeroen Demeyer : -- pull_requests: +11407, 11408 ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue35707] time.sleep() should support objects with __float__

Change by Jeroen Demeyer : -- pull_requests: +11407 ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue35707] time.sleep() should support objects with __float__

Ronald Oussoren added the comment: > In other words: if we can only use __float__ and __int__, how do we know > which one to use? I guess __index__. I've read the definition of object.__index__ in the data model documentation, and using __index__ for this conversion is fine. I need to

[issue35707] time.sleep() should support objects with __float__

Jeroen Demeyer added the comment: > If we want to support other numerical types with loss in double rounding Looking at the existing code, I can already see several double-rounding "bugs" in the code, so I wouldn't be too much concerned here... --

[issue35707] time.sleep() should support objects with __float__

Jeroen Demeyer added the comment: In other words: if we can only use __float__ and __int__, how do we know which one to use? -- ___ Python tracker ___

[issue35707] time.sleep() should support objects with __float__

Jeroen Demeyer added the comment: If __index__ doesn't "feel" right, what do you propose then to fix this issue, keeping in mind the concerns of https://bugs.python.org/issue35707#msg333401 -- ___ Python tracker

[issue35707] time.sleep() should support objects with __float__

Ronald Oussoren added the comment: Using __index__ here doesn't feel right, although I can't explain why yet. -- ___ Python tracker ___

[issue35707] time.sleep() should support objects with __float__

Jeroen Demeyer added the comment: The motivation for PEP 357 was certainly using an object as the index for a sequence, but that's not the only use case. In fact PEP 357 states "For example, the slot can be used any time Python requires an integer internally" So despite the name __index__,

[issue35707] time.sleep() should support objects with __float__

Ronald Oussoren added the comment: As a late response to msg333416 and msg333547, I don't agree with looking at __index__ in _PyTime_FromObject. The __index__ method is used when an object can be used as the index for a sequence, but should not silently convert to int or float. See

[issue35707] time.sleep() should support objects with __float__

Jeroen Demeyer added the comment: To avoid code duplication, it's tempting to merge _PyTime_FromObject and _PyTime_ObjectToDenominator These two functions almost do the same, but not quite. -- ___ Python tracker

[issue35707] time.sleep() should support objects with __float__

STINNER Victor added the comment: No please don't wait for my PR 11507. I'm not sure that it's correct, and this bug is more important than NaN/inf :-) -- ___ Python tracker

[issue35707] time.sleep() should support objects with __float__

Jeroen Demeyer added the comment: I guess I should wait until PR 11507 is merged, to avoid merge conflicts. -- ___ Python tracker ___

[issue35707] time.sleep() should support objects with __float__

STINNER Victor added the comment: > 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. -- ___ Python tracker

[issue35707] time.sleep() should support objects with __float__

Jeroen Demeyer added the comment: 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. -- ___ Python tracker

[issue35707] time.sleep() should support objects with __float__

Jeroen Demeyer added the comment: > 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. -- ___

[issue35707] time.sleep() should support objects with __float__

Jeroen Demeyer added the comment: > 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

[issue35707] time.sleep() should support objects with __float__

Serhiy Storchaka added the comment: 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

[issue35707] time.sleep() should support objects with __float__

Nick Coghlan added the comment: 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

[issue35707] time.sleep() should support objects with __float__

Jeroen Demeyer added the comment: > 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

[issue35707] time.sleep() should support objects with __float__

STINNER Victor added the comment: PR 11507 is not directly related to this issue, see bpo-26669. But I wrote this PR when trying to fix this issue :-) -- ___ Python tracker

[issue35707] time.sleep() should support objects with __float__

Change by STINNER Victor : -- keywords: +patch, patch pull_requests: +11053, 11054 stage: -> patch review ___ Python tracker ___

[issue35707] time.sleep() should support objects with __float__

Change by STINNER Victor : -- keywords: +patch pull_requests: +11053 stage: -> patch review ___ Python tracker ___ ___

[issue35707] time.sleep() should support objects with __float__

STINNER Victor added the comment: 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

[issue35707] time.sleep() should support objects with __float__

Rémi Lapeyre added the comment: See #33039 for the proposed change to __int__. -- ___ Python tracker ___ ___ Python-bugs-list

[issue35707] time.sleep() should support objects with __float__

Change by Rémi Lapeyre : -- nosy: +ncoghlan ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue35707] time.sleep() should support objects with __float__

Rémi Lapeyre added the comment: 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__

[issue35707] time.sleep() should support objects with __float__

AVINASH MISHRA added the comment: hey i am a total newbie to open source contribution. can you help me understand this issue and can i help solve this issue? -- nosy: +AVINASH MISHRA ___ Python tracker

[issue35707] time.sleep() should support objects with __float__

New submission from Jeroen Demeyer : 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