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

[Cheryl Sabella]

Change by Cheryl Sabella :


--
nosy: +belopolsky, p-ganssle
versions: +Python 3.10 -Python 3.6, Python 3.7, Python 3.8

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[STINNER Victor]

STINNER Victor  added the comment:

See also bpo-20861.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Jeroen Demeyer]

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 
https://discuss.python.org/t/pep-3141-ratio-instead-of-numerator-denominator/2037/24?u=jdemeyer
 for a proposal to define __ratio__ for this.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Jeroen Demeyer]

Jeroen Demeyer  added the comment:

Is anybody willing to review PR 11636?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Jeroen Demeyer]

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 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Jeroen Demeyer]

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 chances of PR 11636 being accepted? Unless a 
core developer who is willing to merge that PR asks me that, I'd rather not add 
extra complications to that PR. (to be clear: I mean no offense, it's just that 
getting a CPython PR accepted is hard)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Josh Rosenberg]

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 level, initialized to NULL, and Py_XDECREF it along 
the convert_from_int code path (you can't DECREF it in the index specific path 
because it needs to survive past the goto, since it's replacing obj).

I'm also mildly concerned by how duplicative the code becomes post-patch. If 
it's not a major performance hit (don't think it is; not even sure the API is 
even used anymore), perhaps just implement _PyTime_ObjectToTime_t as a wrapper 
for _PyTime_ObjectToDenominator (with a denominator of 2, so rounding 
simplifies to just 0 == round down, 1 == round up)?

Example:

int
_PyTime_ObjectToTime_t(PyObject *obj, time_t *sec, _PyTime_round_t round)
{
long numerator;
if (_PyTime_ObjectToDenominator(obj, sec, , 2, round) == 0) {
   if (numerator) {
   if (*sec == _Py_IntegralTypeMax(time_t)) {
   error_time_t_overflow();
   return -1;
   }
   ++*sec;
   }
   return 0;
}
return -1;
}

Sorry for not commenting on GitHub, but my work computer has a broken Firefox 
that GitHub no longer supports properly.

--
nosy: +josh.r

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Jeroen Demeyer]

Change by Jeroen Demeyer :


--
components: +Interpreter Core

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Jeroen Demeyer]

Change by Jeroen Demeyer :


--
pull_requests: +11407, 11408, 11409

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Jeroen Demeyer]

Change by Jeroen Demeyer :


--
pull_requests: +11407, 11408

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Jeroen Demeyer]

Change by Jeroen Demeyer :


--
pull_requests: +11407

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Ronald Oussoren]

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 
educate my sense for when it's right to use this method...

Sorry about the noise.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Jeroen Demeyer]

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...

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Jeroen Demeyer]

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 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Jeroen Demeyer]

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 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Ronald Oussoren]

Ronald Oussoren  added the comment:

Using __index__ here doesn't feel right, although I can't explain why yet.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Jeroen Demeyer]

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__, I think that this is now the de facto standard 
for "convert the object (which is some kind of integer) to a Python int without 
loss of precision".

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Ronald Oussoren]

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 
.

--
nosy: +ronaldoussoren

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Jeroen Demeyer]

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 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[STINNER Victor]

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 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Jeroen Demeyer]

Jeroen Demeyer  added the comment:

I guess I should wait until PR 11507 is merged, to avoid merge conflicts.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[STINNER Victor]

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 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Jeroen Demeyer]

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 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Jeroen Demeyer]

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Jeroen Demeyer]

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 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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Serhiy Storchaka]

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 integer arithmetic.

--
nosy: +serhiy.storchaka

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Nick Coghlan]

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 to me, as that addresses Victor's 
desire to use the most precise conversion available.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Jeroen Demeyer]

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 floating-point numbers
3. __int__

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[STINNER Victor]

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 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[STINNER Victor]

Change by STINNER Victor :


--
keywords: +patch, patch
pull_requests: +11053, 11054
stage:  -> patch review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[STINNER Victor]

Change by STINNER Victor :


--
keywords: +patch
pull_requests: +11053
stage:  -> patch review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[STINNER Victor]

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 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.

--
nosy: +vstinner

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Rémi Lapeyre]

Rémi Lapeyre  added the comment:

See #33039 for the proposed change to __int__.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Rémi Lapeyre]

Change by Rémi Lapeyre :


--
nosy: +ncoghlan

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Rémi Lapeyre]

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__ and __index__.

--
nosy: +remi.lapeyre

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[AVINASH MISHRA]

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 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Jeroen Demeyer]

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 
time.sleep(Half())
TypeError: an integer is required (got type Half)

--
messages: 91
nosy: jdemeyer
priority: normal
severity: normal
status: open
title: time.sleep() should support objects with __float__
versions: Python 3.6, Python 3.7, Python 3.8

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com