classification
Title: Add as_integer_ratio() to int() objects
Type: enhancement Stage: resolved
Components: Library (Lib) Versions: Python 3.8
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: lisroach Nosy List: Eric.Wieser, Nofar Schnider, lisroach, mark.dickinson, njs, rhettinger, serhiy.storchaka, tim.peters, vstinner
Priority: normal Keywords: patch

Created on 2018-03-13 21:25 by rhettinger, last changed 2018-10-19 21:46 by vstinner. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 8750 merged lisroach, 2018-08-12 23:37
PR 9297 merged rhettinger, 2018-09-14 07:20
PR 9300 merged rhettinger, 2018-09-14 08:09
PR 9303 merged serhiy.storchaka, 2018-09-14 10:58
Messages (26)
msg313780 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2018-03-13 21:25
Goal:  make int() more interoperable with float by making a float/Decimal method also available on ints.  This will let mypy treat ints as a subtype of floats.

See: https://mail.python.org/pipermail/python-dev/2018-March/152384.html

Open question:  Is this also desired for fractions.Fraction and numbers.Rational?
msg313781 - (view) Author: Tim Peters (tim.peters) * (Python committer) Date: 2018-03-13 21:36
> Is this also desired for fractions.Fraction and numbers.Rational?

I think so.  The idea, e.g., that "it's obvious" for Fraction is no more compelling than that it's even more obvious for ints ;-)  Given that it's spreading to ints anyway, there's an opportunity to make it possible for clients to write utterly uniform code for every type whose values can be represented as integer ratios.

I'd also say it's desirable to extend the Fraction constructor, to accept argument(s) of any type(s) that implement as_integer_ratio().  But that should probably be a different issue.
msg313782 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2018-03-13 21:39
On the Fraction constructor, it looks like there's some overlap with the goals of issue #28716 here.
msg313783 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2018-03-13 21:45
> it looks like there's some overlap with the goals of issue #28716

Okay, let's focus this issue on just int.as_integer_ratio() so that Nofar can have something small and self contained to work on :-)
msg313784 - (view) Author: Nofar Schnider (Nofar Schnider) * (Python triager) Date: 2018-03-13 21:46
I'm working on it
msg313785 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-03-13 21:52
Wouldn't be better to add the function as_integer_ration() in the math module (or in more appropriate place)?

def as_integer_ration(x):
    if hasattr(x, 'as_integer_ration'):
        return x.as_integer_ration()
    else:
        return (x.numerator, x.denominator)

The advantage over adding the int method is that it will automatically support other rational numbers like NumPy integers.
msg313788 - (view) Author: Tim Peters (tim.peters) * (Python committer) Date: 2018-03-13 22:24
Serhiy, we already went down the path of implementing this as a method.   Of course `numbers.Rational` could define the method as `return (self.numerator, self.denominator)` for itself and its subclasses.  Unless I'm confused, that would "magically" define the method for (at least) ints and Fractions too - but I didn't try it.
msg313791 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2018-03-13 22:31
Actually numbers.Rational is a virtual base class for int, so it won't automagically appear there.

Adding it to the math module is inferior because for non-rational types (e.g. alternative float implementations) the math module won't have the knowledge about internals to implement it -- and casting to float() would defeat the purpose.
msg313794 - (view) Author: Tim Peters (tim.peters) * (Python committer) Date: 2018-03-13 23:26
Thanks, Guido!  I figured I was missing something :-)

It looks like `numbers.Rational` _is_ a "for real" base class of `fractions.Fraction`, though, so I'm in favor of supplying a default implementation of `.as_integer_ratio()` in `numbers.Rational` anyway.  That would make a clear statement of intent, and would do the right thing for `Fraction` instances.
msg313795 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2018-03-13 23:43
+1.

On Tue, Mar 13, 2018, 16:26 Tim Peters <report@bugs.python.org> wrote:

>
> Tim Peters <tim@python.org> added the comment:
>
> Thanks, Guido!  I figured I was missing something :-)
>
> It looks like `numbers.Rational` _is_ a "for real" base class of
> `fractions.Fraction`, though, so I'm in favor of supplying a default
> implementation of `.as_integer_ratio()` in `numbers.Rational` anyway.  That
> would make a clear statement of intent, and would do the right thing for
> `Fraction` instances.
>
> ----------
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <https://bugs.python.org/issue33073>
> _______________________________________
>
msg313877 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-03-15 11:51
Adding as_integer_ratio() to numbers.Rational is a breaking change. It breaks the interface. Currently all numbers.Rational subclasses implement all public methods and properties of the abstract class, but with adding as_integer_ratio() this will be no longer true.

>>> issubclass(numpy.int64, numbers.Rational)
True
>>> numpy.int64.as_integer_ratio
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: type object 'numpy.int64' has no attribute 'as_integer_ratio'

I suggest to make as_integer_ratio() creating an implicit interface (as write() creates an implicit interface for writable file-like objects), and add a convenient function (in the numbers or the math module) which calls this method if it exists and falls back to the (nominator, denominator) pair otherwise.
msg313891 - (view) Author: Tim Peters (tim.peters) * (Python committer) Date: 2018-03-15 16:00
Serhiy, I don't understand.  If `numbers.Rational` is in fact a superclass of `numpy.int64`, then the latter will inherit an implementation added to the former.  The idea here isn't to add an abstract method to the Rational interface, but a concrete default implementation:

class Rational(Real):
    ...
    def as_integer_ratio(self):
        return (self.numerator, self.denominator)

Or, as for Python ints, is Rational a "make believe" (virtual) superclass of numpy.int64?
msg313892 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2018-03-15 16:16
It's a "virtual" subclass. The numpy.integer parent class is registered here:

https://github.com/numpy/numpy/blob/10ccfe747a68d974ff16a5c0765054b816d5486f/numpy/core/numerictypes.py#L944
msg313896 - (view) Author: Tim Peters (tim.peters) * (Python committer) Date: 2018-03-15 17:12
Thanks, Mark!  So if int.as_integer_ratio is added to the core, numpy.int64 won't magically have it too, regardless of whether we do or don't add an implementation to numbers.Rational.

As an end user, I'd be surprised if numpy.int64 didn't support the same stuff as core ints.  I doubt I'd care at all what numbers.Rational said in cases where it didn't.

But rather than argue about that, what would _you_ like to see done here?
msg313898 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2018-03-15 18:02
[Tim]

> [...] what would _you_ like to see done here?

Given that float.as_integer_ratio and Decimal.as_integer_ratio already exist, and that I don't have a working time machine right now, I'm in favour of adding int.as_integer_ratio purely for duck-typing reasons: I want to be able to use an int where a float is expected, and I _have_ encountered code in the past where I have to think too hard about what precise types are being used, and where that thinking seems as though it shouldn't be necessary. For the same reason, I'd support as_integer_ratio on the Fraction type. I don't really care whether it gets there via the Rational ABC or not. Adding as_integer_ratio to Rational as an @abstractmethod _does_ seem a step too far (because now anyone who registers a type with numbers.Integral or numbers.Rational has to implement as_integer_ratio), but my understanding is that that's not what's being proposed.

It doesn't bother me that the NumPy integer types won't support as_integer_ratio immediately (or possibly ever). The floating-point types don't either (except np.float64, which only has it because it inherits directly from Python's float), and it's hard to see how as_integer_ratio could be useful for a NumPy float32 array (for example), given that the returned numerator and denominator could exceed the bounds of any of the fixed-width NumPy integer types. I write a lot of NumPy-using code, but the world in which I do so rarely meets the world where I care about correct rounding, fractions, counting ulps error, etc. I haven't missed float32.as_integer_ratio or float16.as_integer_ratio yet.

If we're worried about NumPy compatibility, it might be interesting to get Nathaniel Smith's opinion.
msg313937 - (view) Author: Nathaniel Smith (njs) * (Python committer) Date: 2018-03-16 08:20
Eric Wieser (added to CC) actually just opened a PR for this against NumPy: https://github.com/numpy/numpy/pull/10741

I have weak and mixed feelings about the whole thing: https://github.com/numpy/numpy/pull/10741#issuecomment-373637440
msg313939 - (view) Author: Nathaniel Smith (njs) * (Python committer) Date: 2018-03-16 08:21
Sorry, I misspoke -- I meant he opened a related PR. The PR is to add as_integer_ratio to np.float16, np.float32, np.longdouble, not to add it to the numpy integer types. There are similar issues though.
msg321538 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-07-12 10:51
I remove the "easy (C)" keyword since the feature seems  to be controversal (at least, not so easy).
msg321637 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2018-07-13 21:49
I assume it's decided what to do -- it may not be easy to do it, but I wouldn't call it controversial at this point.
msg321719 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-07-16 08:58
> I assume it's decided what to do -- it may not be easy to do it, but I wouldn't call it controversial at this point.

Ok, I delete "controversal" from my comment :-)

To tag an issue as "easy" (or "easy C"), please describe step by step how you would implement the feature: modified files, modified functions, etc.

It's ok to not give these explanation, but in that case, it's a regular issue, not an easy one.

Note: I'm triagging old "easy" issues. I remove the keyword when I consider that the issue is not easy enough for a newcomer.
msg321729 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2018-07-16 09:59
I put the easy tag here because it was suitable for our mentees and because it is isn't a very difficult task (literally, this is the definition of "easy").

FWIW, it is not "triaging" when the tag was set by a seasoned core developer who was familiar with the issue and already working with a mentee.  Triaging is making an initial assessment when no one else has looked at an issue.
msg321739 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2018-07-16 15:25
Nofar, do you want to continue to work on this or should I reassign?
msg325328 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2018-09-14 06:56
New changeset 5ac704306f4b81ae3f28d8742408d3214b145e8a by Raymond Hettinger (Lisa Roach) in branch 'master':
bpo-33073: Adding as_integer_ratio to ints. (GH-8750)
https://github.com/python/cpython/commit/5ac704306f4b81ae3f28d8742408d3214b145e8a
msg325336 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-09-14 08:42
The merged PR contains several errors. See comments on GitHub.
msg325338 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2018-09-14 09:42
> The merged PR contains several errors. See comments on GitHub.

Some comments were seen after the merge of 8750 and were addressed in 9297.  If you want to change the checked in code, please open a separate PR and tracker item.  If you want different doc markup, please just fix it.  This issue has already sprawled out of proportion to its usefulness and has eaten many hours of our time.
msg328064 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-10-19 21:46
New changeset b2e2025941f6a4fdb716bd141d31acf720353d21 by Victor Stinner (Serhiy Storchaka) in branch 'master':
bpo-33073: Rework int.as_integer_ratio() implementation (GH-9303)
https://github.com/python/cpython/commit/b2e2025941f6a4fdb716bd141d31acf720353d21
History
Date User Action Args
2018-10-19 21:46:35vstinnersetnosy: + vstinner
messages: + msg328064
2018-09-14 10:58:24serhiy.storchakasetpull_requests: + pull_request8732
2018-09-14 09:42:49rhettingersetstatus: open -> closed
resolution: fixed
messages: + msg325338

stage: resolved
2018-09-14 08:42:07serhiy.storchakasetstatus: closed -> open
resolution: fixed -> (no value)
messages: + msg325336

stage: resolved -> (no value)
2018-09-14 08:23:16rhettingersetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2018-09-14 08:09:01rhettingersetpull_requests: + pull_request8729
2018-09-14 07:20:38rhettingersetpull_requests: + pull_request8726
2018-09-14 06:56:27rhettingersetmessages: + msg325328
2018-08-12 23:37:06lisroachsetkeywords: + patch
stage: patch review
pull_requests: + pull_request8230
2018-07-24 04:18:10rhettingersetassignee: Nofar Schnider -> lisroach

nosy: + lisroach
2018-07-16 15:25:21rhettingersetmessages: + msg321739
2018-07-16 14:48:11vstinnersetnosy: - vstinner
2018-07-16 09:59:36rhettingersetmessages: + msg321729
2018-07-16 08:59:14vstinnersetkeywords: - easy (C)
2018-07-16 08:58:36vstinnersetmessages: + msg321719
2018-07-13 21:49:15gvanrossumsetnosy: - gvanrossum
2018-07-13 21:49:09gvanrossumsetmessages: + msg321637
2018-07-12 10:51:05vstinnersetnosy: + vstinner
messages: + msg321538
2018-03-16 08:21:35njssetmessages: + msg313939
2018-03-16 08:20:52njssetnosy: + Eric.Wieser
messages: + msg313937
2018-03-15 18:02:39mark.dickinsonsetnosy: + njs
messages: + msg313898
2018-03-15 17:12:13tim.peterssetmessages: + msg313896
2018-03-15 16:16:58mark.dickinsonsetmessages: + msg313892
2018-03-15 16:00:19tim.peterssetmessages: + msg313891
2018-03-15 11:51:28serhiy.storchakasetmessages: + msg313877
2018-03-13 23:43:24gvanrossumsetmessages: + msg313795
2018-03-13 23:26:20tim.peterssetmessages: + msg313794
2018-03-13 22:31:40gvanrossumsetmessages: + msg313791
2018-03-13 22:24:22tim.peterssetmessages: + msg313788
2018-03-13 21:54:59rhettingersetkeywords: + easy (C)
2018-03-13 21:52:44serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg313785
2018-03-13 21:46:15Nofar Schnidersetmessages: + msg313784
2018-03-13 21:45:02rhettingersetmessages: + msg313783
2018-03-13 21:39:13mark.dickinsonsetmessages: + msg313782
2018-03-13 21:37:35mark.dickinsonsetnosy: + mark.dickinson
2018-03-13 21:36:37tim.peterssetnosy: + tim.peters
messages: + msg313781
2018-03-13 21:25:05rhettingercreate