This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Integral.denominator shouldn't return an int
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.11
process
Status: closed Resolution: not a bug
Dependencies: Superseder:
Assigned To: Nosy List: Sergey.Kirpichev, lemburg, mark.dickinson, rhettinger, serhiy.storchaka, stutzbach, terry.reedy
Priority: normal Keywords: patch

Created on 2022-03-21 03:48 by Sergey.Kirpichev, last changed 2022-04-11 14:59 by admin. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 32022 closed Sergey.Kirpichev, 2022-03-21 07:01
Messages (15)
msg415660 - (view) Author: Sergey B Kirpichev (Sergey.Kirpichev) * Date: 2022-03-21 03:48
As the __int__() method is required, I believe it can return self(1) instead.

Also, probably +self normalization could be avoided in the Integral.numerator().
msg415664 - (view) Author: Sergey B Kirpichev (Sergey.Kirpichev) * Date: 2022-03-21 04:28
> self(1) instead

or, rather, type(self)(1)
msg416038 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2022-03-26 05:34
Raymond and Serhiy, you were the last two active devs to touch numbers.py.
msg416039 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2022-03-26 05:40
How would it work for bool or IntEnum?
msg416041 - (view) Author: Sergey B Kirpichev (Sergey.Kirpichev) * Date: 2022-03-26 06:10
> How would it work for bool or IntEnum?

Neither subclass the Integral abc.

The constraint (for which I care about) is: numerator/denominator should have same types.  The default implementation, which I propose: it's the same type as the given Integral implementation.
msg416044 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2022-03-26 07:07
There is no such constrain. And no default implementation in this module depends on the constructor. It is important, the constructor is not the part of interfaces.

If you want to return the same type in denominator, just override it in your class.
msg416046 - (view) Author: Sergey B Kirpichev (Sergey.Kirpichev) * Date: 2022-03-26 07:15
> There is no such constrain.

Why not?  Any example where it does make sense to have different types for numerator and denominator?
msg416047 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2022-03-26 07:36
There is no "why". There is a fact that there is no such constrain. Adding a new constrain may break existing code. If you want to add a constrain, add it in you code.

If you are interesting "why", try to search old archives for the history of creating that module. Maybe you will find something. Maybe you will need to dip deeper, in discussions about abstract classes and protocols.
msg416049 - (view) Author: Sergey B Kirpichev (Sergey.Kirpichev) * Date: 2022-03-26 08:21
> There is no "why".

Why not?  Apparently, this is a documentation error, at least.  (And I doubt there are tests for default methods.)

> If you are interesting "why", try to search old archives for the history of creating that module.

Thanks.  Sorry, I'll try harder.  So far I've found nothing to explain me why the Integral.denominator must be an 'int'.
msg416054 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2022-03-26 11:13
The module documentation should not contain all historical reasons of every design decision. If you are interesting why something was done in one way or another, do your research.

One of reasons is that type(self) not always have a constructor with compatible interface. So it may work for your integer-like type, but in general case.
msg416056 - (view) Author: Sergey B Kirpichev (Sergey.Kirpichev) * Date: 2022-03-26 11:59
> The module documentation should not contain all historical reasons of every design decision.

Sure.  But, for example, there should be an explanation of why foo+foo.denominator could produce an error for a valid implementation of the Integral abc.  Such interoperability is not assumed by the docs, according to provided examples (https://docs.python.org/3/library/numbers.html#implementing-the-arithmetic-operations).

> One of reasons is that type(self) not always have a constructor with compatible interface.

I hardly can imagine a constructor for the Integral type, which doesn't accept a python integer as an argument.
msg416059 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2022-03-26 12:30
Integral.__add__ is an abstract method, so it is a problem of your implementation.
msg416102 - (view) Author: Sergey B Kirpichev (Sergey.Kirpichev) * Date: 2022-03-27 03:32
> Integral.__add__ is an abstract method, so it is a problem of your implementation.

But such an implementation does satisfy all assumptions of the Integral abc, correct?
msg416106 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2022-03-27 06:45
It does not satisfy your assumptions in msg416056. So you have either correct your assumptions, or change the implementation of __add__, or change the implementation of denominator in your code.
msg416117 - (view) Author: Sergey B Kirpichev (Sergey.Kirpichev) * Date: 2022-03-27 10:38
> It does not satisfy your assumptions in msg416056.

Yes, but does it fits to assumptions of the numbers module?  To be fair, there is even no requirement, that numerator/denominator are Integral instances (#25619 address also this).

BTW, it seems, the numpy integer types break my guess that numerator and denominator must have same types (I doubt that anyone uses numbers.Integral defaults, there are no tests for them in the CPython itself):
>>> a = numpy.int8(2); a
2
>>> type(a)
<class 'numpy.int8'>
>>> type(a.numerator)
<class 'numpy.int8'>
>>> type(a.denominator)
<class 'int'>
>>> type(a + a.denominator)
<class 'numpy.int64'>
>>> type(a.denominator + a)
<class 'numpy.int64'>
>>> type(a + a)
<class 'numpy.int8'>

If this is an expected behaviour for Integral types - feel free to close the bugreport.  Looks odd for me, however.
History
Date User Action Args
2022-04-11 14:59:57adminsetgithub: 91235
2022-03-27 11:19:43serhiy.storchakasetstatus: open -> closed
resolution: not a bug
stage: patch review -> resolved
2022-03-27 10:38:19Sergey.Kirpichevsetmessages: + msg416117
2022-03-27 06:45:35serhiy.storchakasetmessages: + msg416106
2022-03-27 03:32:28Sergey.Kirpichevsetmessages: + msg416102
2022-03-26 12:30:53serhiy.storchakasetmessages: + msg416059
2022-03-26 11:59:41Sergey.Kirpichevsetmessages: + msg416056
2022-03-26 11:13:44serhiy.storchakasetmessages: + msg416054
2022-03-26 08:21:45Sergey.Kirpichevsetmessages: + msg416049
2022-03-26 07:36:59serhiy.storchakasetmessages: + msg416047
2022-03-26 07:15:42Sergey.Kirpichevsetmessages: + msg416046
2022-03-26 07:07:13serhiy.storchakasetmessages: + msg416044
2022-03-26 06:10:09Sergey.Kirpichevsetmessages: + msg416041
2022-03-26 05:40:51serhiy.storchakasetnosy: + lemburg, mark.dickinson, stutzbach
messages: + msg416039
2022-03-26 05:34:47terry.reedysetnosy: + rhettinger, terry.reedy, serhiy.storchaka
messages: + msg416038
2022-03-21 07:01:16Sergey.Kirpichevsetkeywords: + patch
stage: patch review
pull_requests: + pull_request30110
2022-03-21 04:28:49Sergey.Kirpichevsetmessages: + msg415664
2022-03-21 03:48:09Sergey.Kirpichevcreate