classification
Title: float is missing __ceil__() and __floor__(), required by numbers.Real
Type: behavior Stage: resolved
Components: Interpreter Core, Library (Lib) Versions: Python 3.9
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: BTaskaya, bluetech, mark.dickinson, serhiy.storchaka, veky, vstinner
Priority: normal Keywords: patch

Created on 2019-10-29 10:18 by bluetech, last changed 2019-12-15 22:01 by vstinner. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 16985 merged BTaskaya, 2019-10-29 14:37
Messages (11)
msg355642 - (view) Author: Ran Benita (bluetech) Date: 2019-10-29 10:18
The numbers.Real ABC requires the __ceil__ and __floor__ methods (https://github.com/python/cpython/blob/v3.8.0/Lib/numbers.py#L178-L186), however, the float type does not have them.

In regular Python, this is not a problem, because math.ceil() and math.floor() special-case float and work on it directly, and numbers.Real is implemented on float by explicitly registering it (so it's not checked that all required methods are implemented).

Where it becomes a (minor) problem is in typeshed, where the type checker *does* check that all abstract methods are implemented. So, because float is missing these two, typeshed cannot have float implement numbers.Real.

Refs: https://github.com/python/typeshed/issues/3195
msg355648 - (view) Author: Batuhan Taskaya (BTaskaya) * (Python committer) Date: 2019-10-29 12:29
Ran, do you want to work on this or can i take it?
msg355650 - (view) Author: Ran Benita (bluetech) Date: 2019-10-29 12:31
You can take it - thanks!
msg355668 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2019-10-29 18:48
If float.__ceil__ is only needed for typeshed, maybe add a special case for typeshed?

I have doubts about adding a C code which is never even executed.
msg355670 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2019-10-29 19:01
> I have doubts about adding a C code which is never even executed.

My reading of the PR is that it *would* be executed: the math module first looks for the __floor__ special method, then falls back to using the libm floor if that doesn't exist. Am I missing something?
msg355673 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2019-10-29 19:42
Oh, you are right. I misunderstood the original issue and thought that the code first checks PyFloat_Check() or PyFloat_CheckExact().

If float.__ceil__ will be executed it will add significant overhead for creating and executing the method object.
msg355675 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2019-10-29 20:24
> If float.__ceil__ will be executed it will add significant overhead for creating and executing the method object.

Yes, I'd definitely like to see timings; I think Victor already asked for those on the PR.
msg355722 - (view) Author: Batuhan Taskaya (BTaskaya) * (Python committer) Date: 2019-10-30 16:12
$ ./python -m pyperf timeit -s "from math import floor" --duplicate 100 "floor(12345.6)"
Before:  Mean +- std dev: 52.5 ns +- 2.6 ns
After:  Mean +- std dev: 71.0 ns +- 1.7 ns

$ ./python -m pyperf timeit -s "from math import ceil" --duplicate 100 "ceil(12345.6)"
Before:  Mean +- std dev: 51.2 ns +- 1.5 ns
After:  Mean +- std dev: 74.4 ns +- 2.2 ns
msg355744 - (view) Author: Vedran Čačić (veky) * Date: 2019-10-31 10:54
However, this is an instance of a general problem: whenever we want to strongly type (via dunders) protocols that specialcase builtin types, we will have to choose between three options:

* special case them also in typing engine, complicating the typing engine
* implement dummy dunders, puzzling readers of the code
* implement dunders that do the right thing but never actually execute, puzzling Serhiy (and probably others)
* implement dunders that are actually called (un-specialcasing builtin types), slowing down the common path

Do we have a preference for a "default" position when we encounter such problems in the future? Of course, we can override it on a case-by-case basis in the presence of good arguments, but still, a default would be nice to have.

I don't know much about static typing (which is why I loved Python until this typing craze happened:), but it seems to me that we might have another option: we can currently say that a type might be a virtual subclass of an abstract class in more than one way, right? For example, we still support old-style iterators (via __getitem__ and IndexError), IIRC. So, can we say that a type can implement numbers.Real also in two ways: by having some dunders, or by being (a literal or a subtype of) float?
msg358452 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-12-15 22:00
New changeset cb8b946ac10386e6cab1376945f64f683b5b16d3 by Victor Stinner (Batuhan Taşkaya) in branch 'master':
bpo-38629: implement __floor__ and __ceil__ for float type (GH-16985)
https://github.com/python/cpython/commit/cb8b946ac10386e6cab1376945f64f683b5b16d3
msg358453 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-12-15 22:01
Thanks Batuhan Taşkaya for the implementation, and thanks Ran Benita for the feature request :-)
History
Date User Action Args
2019-12-15 22:01:27vstinnersetstatus: open -> closed
versions: + Python 3.9
messages: + msg358453

resolution: fixed
stage: patch review -> resolved
2019-12-15 22:00:42vstinnersetnosy: + vstinner
messages: + msg358452
2019-10-31 10:54:54vekysetnosy: + veky
messages: + msg355744
2019-10-30 16:12:03BTaskayasetmessages: + msg355722
2019-10-29 20:24:55mark.dickinsonsetmessages: + msg355675
2019-10-29 19:42:34serhiy.storchakasetmessages: + msg355673
2019-10-29 19:01:24mark.dickinsonsetmessages: + msg355670
2019-10-29 18:48:02serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg355668
2019-10-29 14:37:08BTaskayasetkeywords: + patch
stage: patch review
pull_requests: + pull_request16511
2019-10-29 12:58:55xtreaksetnosy: + mark.dickinson
2019-10-29 12:31:58bluetechsetmessages: + msg355650
2019-10-29 12:29:14BTaskayasetnosy: + BTaskaya
messages: + msg355648
2019-10-29 10:18:17bluetechcreate