PEP 3141: __ratio__ instead of numerator/denominator

But it would solve it from an ABC-interface perspective, no? If the sage devs want Integer to follow not just the letter but also the spirit of the PEP, they would have to change the API.

Or do you envision that both i.denominator() and i.denominator return 1? Re:

And yet you seem to be happy with suggesting that Python break backwards compatibility? It sounds like there’s no way of unifying things without one or the other project changing.

(I’m speaking here as someone who doesn’t use SageMath, and has code that uses Rational.numerator, so I clearly have a bias ;-))

My proposal can be implemented in an entirely backwards compatible way (I know that it would be immediately rejected otherwise). Supporting __denominator__ does not require removing support for denominator.

OK. I don’t understand how, from what you’ve written, but I;ll take your word for it. As long as I can continue using the property Rational.numerator to get the numerator of a fraction, that’s fine for my code.

[quote=“steven.daprano, post:2, topic:2037”]

Does denominator need to be a method in SageMath? Why couldn’t they have

used a property?

[/quote]

That doesn’t really matter. Maybe SageMath could have used a property,

but they didn’t. It’s just a choice, there is nothing wrong with a

method.

It matters in case SageMath need to pass arguments to the denominator()

method. That would seriously limit their ability to change to a

property.

It matters because SageMath, or rather you on behalf of SageMath, are

asking us to change an API for the entire Python ecosystem, likely

breaking masses of code, so that they can meet the Rational ABC without

changing their APIs. Why should we ask everyone else to change their

code to save SageMath a bit of effort?

[quote=“steven.daprano, post:2, topic:2037”]

they should just deprecate the current behaviour and use a property.

[/quote]

I don’t see how to do that technically. What kind of object would q.denominator then be?

I don’t understand your question. If I take it literally, it implies

that you don’t know what properties are, and surely that’s not the case.

q.denominator would be a property object, which is a kind of

descriptor, that returns whatever kind of object SageMath wants to

return. Probably an int for numbers, possibly a polynomial object if

they support polynomial division, possibly other things.

Assuming that SageMath wants to support the Rational ABC, that means

they would go through a deprecation period where q.denominator() raises

a warning. At the end of the deprecation period, change the method to a

property.

Surely I don’t need to explain to you how to change a method into a

property? I can’t help but feel we must be talking past each other.

def denominator(self):

    warnings.warn("denominator will stop being a method in version X")

    # implementation goes here

becomes

@property

def denominator(self):

    # implementation goes here

If they want to avoid a hard cut-off from one behaviour to the other,

they could return a value with a call method that returns self. That

way, q.denominator() and q.denominator will both return the same object.

My proposal is meant to be backwards compatible. It’s perfectly possible to support both denominator and __denominator__ at the same time.

For me, a deprecation means a period where the new way already works, while the old way still works with a warning. Since denominator can’t be a method and a property at the same time, this doesn’t work here. That’s really the essence of the problem why SageMath cannot “just” support PEP 3141.

Taking a step back, the reason why Python uses dunder names for special methods is to avoid name conflicts like this. For example, __next__ has a very specific meaning while next could mean anything (not being a reserved name). That’s the deeper reason for this proposal.

That’s probably what we’ll end up doing if this proposal is rejected.

I’d like to modify my proposal slightly: have just one special method __ratio__ (name subject to bikeshedding) returning a pair (numerator, denominator). This would generalize float.as_integer_ratio(), so it would be the special method for converting a number to a fraction.

I’m going to go back on my statement that I’d take your word that this can be done in a backward compatible way. Mainly because I don’t see how you expect __ratio__ to work any better. (Honestly, I think you should flesh out your proposal to be a lot more precise, to avoid this confusion).

If I currently have code:

def my_function(frac: Rational) -> Integral:
    return frac.numerator + frac.denominator

This works today, and can be used for any input that has a type compatible with numbers.Rational.

I don’t see how you expect things to be in the future so that SageMath fractions can be numbers.Rational and yet my code can remain unchanged. If you pass a SageMath rational to my code, it’ll break (because numerator is a method, not a property). And if you can’t pass a SageMath rational to my function, then either you’ve changed the meaning of my type annotation, or SageMath rationals aren’t Rationals

So either we have different ideas of what “backwards compatible” means, or I’ve badly misunderstood your proposal.

2 Likes

Yes, it will break. But that’s not a backwards compatibility issue. That’s code which doesn’t work today and which still won’t work.

Indeed my proposal is changing the meaning of numbers.Rational, consider it an amendment to PEP 3141. So your code will need to be changed to support SageMath rationals. But the point of backwards compatibility is that nothing will break if you don’t change your code.

The code works today. Not with SageMath rationals, certainly, but the type annotation makes that fact clear.

OK. So we’re now clear, this is a change to the currently documented definition of numbers.Rational. Fine - but I wish you’d simply stated this clearly from the start.

Huh? My code will be required to change if its annotation is to remain valid. In what possible way is that “not breaking if I don’t change my code”? Or are you suggesting that breaking annotations is not subject to backward compatibility rules?

And how do I need to change my code so it’ll still work? What do I replace frac.numerator with? Your proposal doesn’t say, it just leaves us to assume that you’re asking me to use __ratio__ (or another dunder method) in user code, which is not normal practice :frowning:

I guess I just disagree with what you’re proposing in the absence of a sufficiently precise proposal. Whether my view will change when you produce a full proposal, I doubt, but we can leave that question until the full proposal is posted.

2 Likes

My first post in this thread talked about changing PEP 3141, so I assumed that this was clear. But it’s good that this misunderstanding is over now.

Just to make it more clear what we’re talking about, could you post a link to that code?

In my view, “backwards compatibility” is about observable functionality. A good proxy for this is: it shouldn’t break any testsuites. By this definition, my proposal can be implemented in a backwards compatible way.

What’s happening here is that my proposal is adding a bug to your code. The fact is and remains that your code (unchanged) doesn’t support SageMath rationals. Right now, your code is not to blame for this. But with the proposal, your code might be blaimed.

No, it’s unpublished personal code. To avoid misunderstanding, I’m not talking about a production system that will break here - obviously I won’t suddenly be adding SageMath rationals to such code. This is a small personal project, mostly experimental (it’s playing about with continued fractions) that could well languish for a while and then I’ll pick it up and run it again. I probably also have some data analysis code from a while back that uses fractions and freely expects to use .numerator etc. All I’m really saying is that I’m a relatively frequent user of fractions in my code, and I can’t assess the impact on my code from the partial proposal you’ve posted this far.

Ultimately, though, I don’t care at this point. Until I see a full proposal, I’m just anticipating problems. Once you have the proposal written up, then I’ll look at it and decide whether I’m comfortable with the breakage (and whether or not I want to argue with you over what counts as “breakage”, if that’s necessary).

Ok, here is a more complete proposal:

  1. Define a new special method __ratio__ with the following meaning: q.__ratio__() must be a 2-tuple (n, d) of integers such that q * d == n. The objects n and d don’t have to be Python ints, but they must support __index__.

  2. Within the standard library, int, fractions.Fraction, float and decimal.Decimal would support __ratio__. For the latter two, this would be an alias of as_integer_ratio. Note that __ratio__ is therefore not limited to numbers.Rational, it can be used also for exact arithmetic with floating point numbers. This may be useful for the time module, see bpo-35707.

  3. The specification of numbers.Rational would be changed to say that rationals must have such a __ratio__ method. The numerator and denominator properties are no longer required (but the expectation is that existing classes will keep them).

  4. numbers.Rational would have a default implementation of __ratio__ returning (self.numerator, self.denominator). Note that this automatically makes __ratio__ work for fractions.Fraction and all other existing classes inheriting from numbers.Rational.

  5. A helper function operator.ratio(x) is added, returning x.__ratio__() but falling back to (x.numerator, x.denominator). This is recommended over calling __ratio__ manually. If it’s deemed useful, also a C API function will be added.

  6. The constructor for fractions.Fraction would use operator.ratio().

As far as I can see, this proposal is backwards compatible in the sense that all functionality that used to work still works. Nevertheless, existing code should be changed to support the new protocol:

  1. Classes registering as numbers.Rational without actually inheriting should implement __ratio__.

  2. Code checking for numbers.Rational and accessing the numerator/denominator properties should use operator.ratio() instead.

1 Like

I don’t consider this backwards compatible. Every user of .numerator etc. would have to be changed. And what’s the point? So SageMath numbers can claim compatibility with PEP 3141? Even the stdlib’s decimal module doesn’t claim to be compatible with the numbers API. I really don’t see the point of making such a change. If you need to pass a SageMath number to some API that expects the numbers API you’ll just have to convert at the boundary.

To use RFC 2119, it’s not REQUIRED for such code to change but it is RECOMMENDED (nothing that used to work will break if code doesn’t change). The first is obviously not backwards compatible, the second is in a gray zone depending on how strictly you define backwards compatibility.

The real goal is interoperability with fractions.Fraction and currently PEP 3141 is the only means to that end. Think of it this way: we have __index__ for converting to a Python int, __float__ to a float and __complex__ to a complex. But we’re missing a special method for fractions.

And what real-world problem does that solve?

2 Likes

There are various mathematical packages that use fractions.Fraction which are actually used by people to do real work. For example, realalg to name just one.

Using such packages inside SageMath is more difficult than it should be because Fraction does not support SageMath integers/rationals. Of course, one can do explicit conversions but that’s not convenient. For integers, __index__ was invented to solve this problem.

It seems now that Python is converging towards using as_integer_ratio for rationals. The only part that’s missing is actually using as_integer_ratio in the Fraction constructor, see either PR 15327 or PR 15329.

3 Likes

Sounds good.