msg313780 - (view) |
Author: Raymond Hettinger (rhettinger) *  |
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) *  |
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) *  |
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) *  |
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) *  |
Date: 2018-03-13 21:46 |
I'm working on it
|
msg313785 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) *  |
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) *  |
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) *  |
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) *  |
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) *  |
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) *  |
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) *  |
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) *  |
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) *  |
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) *  |
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) *  |
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) *  |
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) *  |
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) *  |
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) *  |
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) *  |
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) *  |
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) *  |
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) *  |
Date: 2018-09-14 08:42 |
The merged PR contains several errors. See comments on GitHub.
|
msg325338 - (view) |
Author: Raymond Hettinger (rhettinger) *  |
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) *  |
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
|
|
Date |
User |
Action |
Args |
2022-04-11 14:58:58 | admin | set | github: 77254 |
2018-10-19 21:46:35 | vstinner | set | nosy:
+ vstinner messages:
+ msg328064
|
2018-09-14 10:58:24 | serhiy.storchaka | set | pull_requests:
+ pull_request8732 |
2018-09-14 09:42:49 | rhettinger | set | status: open -> closed resolution: fixed messages:
+ msg325338
stage: resolved |
2018-09-14 08:42:07 | serhiy.storchaka | set | status: closed -> open resolution: fixed -> (no value) messages:
+ msg325336
stage: resolved -> (no value) |
2018-09-14 08:23:16 | rhettinger | set | status: open -> closed resolution: fixed stage: patch review -> resolved |
2018-09-14 08:09:01 | rhettinger | set | pull_requests:
+ pull_request8729 |
2018-09-14 07:20:38 | rhettinger | set | pull_requests:
+ pull_request8726 |
2018-09-14 06:56:27 | rhettinger | set | messages:
+ msg325328 |
2018-08-12 23:37:06 | lisroach | set | keywords:
+ patch stage: patch review pull_requests:
+ pull_request8230 |
2018-07-24 04:18:10 | rhettinger | set | assignee: Nofar Schnider -> lisroach
nosy:
+ lisroach |
2018-07-16 15:25:21 | rhettinger | set | messages:
+ msg321739 |
2018-07-16 14:48:11 | vstinner | set | nosy:
- vstinner
|
2018-07-16 09:59:36 | rhettinger | set | messages:
+ msg321729 |
2018-07-16 08:59:14 | vstinner | set | keywords:
- easy (C) |
2018-07-16 08:58:36 | vstinner | set | messages:
+ msg321719 |
2018-07-13 21:49:15 | gvanrossum | set | nosy:
- gvanrossum
|
2018-07-13 21:49:09 | gvanrossum | set | messages:
+ msg321637 |
2018-07-12 10:51:05 | vstinner | set | nosy:
+ vstinner messages:
+ msg321538
|
2018-03-16 08:21:35 | njs | set | messages:
+ msg313939 |
2018-03-16 08:20:52 | njs | set | nosy:
+ Eric.Wieser messages:
+ msg313937
|
2018-03-15 18:02:39 | mark.dickinson | set | nosy:
+ njs messages:
+ msg313898
|
2018-03-15 17:12:13 | tim.peters | set | messages:
+ msg313896 |
2018-03-15 16:16:58 | mark.dickinson | set | messages:
+ msg313892 |
2018-03-15 16:00:19 | tim.peters | set | messages:
+ msg313891 |
2018-03-15 11:51:28 | serhiy.storchaka | set | messages:
+ msg313877 |
2018-03-13 23:43:24 | gvanrossum | set | messages:
+ msg313795 |
2018-03-13 23:26:20 | tim.peters | set | messages:
+ msg313794 |
2018-03-13 22:31:40 | gvanrossum | set | messages:
+ msg313791 |
2018-03-13 22:24:22 | tim.peters | set | messages:
+ msg313788 |
2018-03-13 21:54:59 | rhettinger | set | keywords:
+ easy (C) |
2018-03-13 21:52:44 | serhiy.storchaka | set | nosy:
+ serhiy.storchaka messages:
+ msg313785
|
2018-03-13 21:46:15 | Nofar Schnider | set | messages:
+ msg313784 |
2018-03-13 21:45:02 | rhettinger | set | messages:
+ msg313783 |
2018-03-13 21:39:13 | mark.dickinson | set | messages:
+ msg313782 |
2018-03-13 21:37:35 | mark.dickinson | set | nosy:
+ mark.dickinson
|
2018-03-13 21:36:37 | tim.peters | set | nosy:
+ tim.peters messages:
+ msg313781
|
2018-03-13 21:25:05 | rhettinger | create | |