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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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.
|
|
Date |
User |
Action |
Args |
2022-04-11 14:59:57 | admin | set | github: 91235 |
2022-03-27 11:19:43 | serhiy.storchaka | set | status: open -> closed resolution: not a bug stage: patch review -> resolved |
2022-03-27 10:38:19 | Sergey.Kirpichev | set | messages:
+ msg416117 |
2022-03-27 06:45:35 | serhiy.storchaka | set | messages:
+ msg416106 |
2022-03-27 03:32:28 | Sergey.Kirpichev | set | messages:
+ msg416102 |
2022-03-26 12:30:53 | serhiy.storchaka | set | messages:
+ msg416059 |
2022-03-26 11:59:41 | Sergey.Kirpichev | set | messages:
+ msg416056 |
2022-03-26 11:13:44 | serhiy.storchaka | set | messages:
+ msg416054 |
2022-03-26 08:21:45 | Sergey.Kirpichev | set | messages:
+ msg416049 |
2022-03-26 07:36:59 | serhiy.storchaka | set | messages:
+ msg416047 |
2022-03-26 07:15:42 | Sergey.Kirpichev | set | messages:
+ msg416046 |
2022-03-26 07:07:13 | serhiy.storchaka | set | messages:
+ msg416044 |
2022-03-26 06:10:09 | Sergey.Kirpichev | set | messages:
+ msg416041 |
2022-03-26 05:40:51 | serhiy.storchaka | set | nosy:
+ lemburg, mark.dickinson, stutzbach messages:
+ msg416039
|
2022-03-26 05:34:47 | terry.reedy | set | nosy:
+ rhettinger, terry.reedy, serhiy.storchaka messages:
+ msg416038
|
2022-03-21 07:01:16 | Sergey.Kirpichev | set | keywords:
+ patch stage: patch review pull_requests:
+ pull_request30110 |
2022-03-21 04:28:49 | Sergey.Kirpichev | set | messages:
+ msg415664 |
2022-03-21 03:48:09 | Sergey.Kirpichev | create | |