Opened 3 years ago

Sage Integers are incompatible with Python fraction.Fraction, statistics.mean, itertools.permutations

Reported by: Owned by: mcbell major sage-9.7 basic arithmetic days100 jdemeyer, mcbell, slelievre, vdelecroix, SimonKing, gh-posita, tscrim Mark Bell N/A public/28234 4977867e55d76c29b7286e2d542d8342889d8305

fraction.Fraction

In Sage 8.8 the code:

```sage: from fractions import Fraction
sage: Fraction(1, 1)
```

raises the following error:

```---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-4-0c4690dff1ca> in <module>()
----> 1 Fraction(Integer(1), Integer(1))

/Applications/SageMath-8.8.app/Contents/Resources/sage/local/lib/python2.7/fractions.pyc in __new__(cls, numerator, denominator)
152             isinstance(denominator, Rational)):
153             numerator, denominator = (
--> 154                 numerator.numerator * denominator.denominator,
155                 denominator.numerator * numerator.denominator
156                 )

TypeError: unsupported operand type(s) for *: 'builtin_function_or_method' and 'builtin_function_or_method'
```

This appears to be because of the fact that Python ints have numerator and denominator properties whereas for Sage integers these are methods. Therefore Fraction does not call these attributes leading to the crash

Note, turning off the pre-parser or doing:

```sage: Fraction(int(1), int(1))
Fraction(1, 1)
```

works.

One solution to this would be to decorate sage.rings.integer.Integer.numerator() and sage.rings.integer.Integer.denominator() with the @property decorator, but this will likely cause a large number of systems to break.

Earlier report of this issue in #10928, comment 14 (a ticket about getitem of numpy matrix using Sage integers).

statistics.mean (see #29662)

Although the `as_integer_ratio` mechanism has been added to Python, ​`statistics._exact_ratio` does not use `as_integer_ratio` because it sees the Sage numbers' `numerator` and `denominator` methods and tries to use them as properties.

itertools.permutations

```┌────────────────────────────────────────────────────────────────────┐
│ SageMath version 9.5.beta7, Release Date: 2021-11-18               │
│ Using Python 3.9.9. Type "help()" for help.                        │
└────────────────────────────────────────────────────────────────────┘
sage: import itertools
sage: list(itertools.permutations(range(3),int(2)))
[(0, 1), (0, 2), (1, 0), (1, 2), (2, 0), (2, 1)]
sage: list(itertools.permutations(range(3),2))
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-8-a9f97c1deeca> in <module>
----> 1 list(itertools.permutations(range(Integer(3)),Integer(2)))

TypeError: Expected int as r
```

comment:1 Changed 3 years ago by mcbell

• Description modified (diff)

comment:2 Changed 3 years ago by mcbell

• Authors set to mcbell
• Description modified (diff)

comment:3 Changed 3 years ago by mcbell

This issue also appears to affect Sage rationals where

```sage: from fractions import Fraction
sage: Fraction(3/4)
Fraction(<built-in method numerator of sage.rings.rational.Rational object at 0x17d185ad0>, <built-in method denominator of sage.rings.rational.Rational object at 0x17d185ad0>)
```

since 3/4 is a sage Rational, which is a numbers.Rational and so during initialisation this block is triggered:

```101         if denominator is None:
102             if isinstance(numerator, Rational):
103                 self._numerator = numerator.numerator
104                 self._denominator = numerator.denominator
105                 return self
```

which sets the numerator and denominator of the fraction to be the numerator and denominator methods of 3/4.

comment:4 Changed 3 years ago by slelievre

• Authors mcbell deleted

comment:5 Changed 3 years ago by mcbell

Note that the https://docs.python.org/3/library/numbers.html says that these should be properties:

```class numbers.Rational
Subtypes Real and adds numerator and denominator properties, which should be in lowest terms. With these, it provides a default for float().

numerator
Abstract.

denominator
Abstract.
```

Similar is true for numbers.Integral

comment:6 Changed 3 years ago by slelievre

Indeed, when Sage number classes were registered into Python numbers abstract base classes (#19571), it was overlooked that the Python numbers abc specifies rationals should have a numerator property and denominator property, while in Sage they are methods that one needs to call.

Some solutions discussed at Sage Days 100:

• do nothing: then users must do different things for Sage numbers or Python numbers
• change Sage, with a deprecation period and temporary fix where calling a Sage integer with no arguments returns this integer, with a warning
• change Python

comment:7 Changed 3 years ago by mcbell

I have attached a potential patch for discussion which:

1. Marks the numerator and denominator methods of Integer and Rational as properties using the property decorator
2. Creates full numer and denom methods of Rational which return these properties
3. Makes integers callable but:
1. Raises a TypeError? (as before) if they are called with anything
2. Generates a sage.misc.superseded.deprecation warning referring to this ticket and returns self if they are called without any arguments.

This means that if `x = 7` then doing `x.numerator` returns `7` while doing `x.numerator()` is evaluated via `7()` which, thanks to case 3b, generates a deprecation and then returns `7`. A similar process happens if `x = 7/3`, although in this case no deprecation warning is raised when using the `x.denom()` which is provided as an alias.

Last edited 3 years ago by mcbell (previous) (diff)

Changed 3 years ago by mcbell

Patch to make numerator and denominator properties

comment:8 Changed 3 years ago by vdelecroix

• Branch set to public/28234
• Commit set to 6756542a807a5fc00252e4564ea4a550d67a1a45

New commits:

 ​6756542 `make numerator/denominator properties`

comment:9 follow-up: ↓ 10 Changed 3 years ago by vdelecroix

a good point:

• It indeed solves the problem and remains backward compatible

Some problems:

• It affects a *lot* of files inside the Sage source code (at least 142 present doctest error) - this is easily solvable by patching the said files. But this kind of patch bomb might overlap with ongoing work of other people.
• There are several other objects with a `numerator/denominator` method in Sage. Having integers and rationals behaving differently than the rest of Sage would be really weird. A solution would be: just do the same for other objects. But then the patch bomb becomes even bigger...

comment:10 in reply to: ↑ 9 Changed 3 years ago by mcbell

Some problems:

• It affects a *lot* of files inside the Sage source code (at least 142 present doctest error) - this is easily solvable by patching the said files. But this kind of patch bomb might overlap with ongoing work of other people.

Regarding this, these appear to come in two flavours:

1. The testing framework is not happy with deprecation warnings,
2. This now makes the Sage Integer class callable and it appears that there are doctest that determine whether an object is an integer or not by testing whether it is callable!

While 1 could be solved by just removing the warning, 2 will involve making changes to hundreds of tests.

comment:13 in reply to: ↑ 11 Changed 3 years ago by SimonKing

For an explicit example of the previous behaviour see: https://github.com/sagemath/sage/blob/master/src/sage/rings/polynomial/infinite_polynomial_element.py#L1523

Cool. I didn't recall that I had used callability at some point...

comment:14 follow-up: ↓ 15 Changed 3 years ago by jdemeyer

I'm not sure that such a hack is worth it. We could at least ask Python upstream. One could argue that Python should use special methods for this like `__numerator__`.

comment:15 in reply to: ↑ 14 ; follow-up: ↓ 27 Changed 3 years ago by vdelecroix

I'm not sure that such a hack is worth it. We could at least ask Python upstream. One could argue that Python should use special methods for this like `__numerator__`.

It makes sense. Would you make such a proposal? Note that it affects Python but also `numpy`.

comment:16 Changed 3 years ago by mcbell

Another suggestion that could be considered, is to make numerator and denominator properties throughout Sage. Obviously such a change would not be backward compatible. Therefore it may have a major impact on external code that others have written - which must be taken into consideration.

However, within /src/sage/:

1. There are only only 22 instances of "def numerator(" and 36 instances of "def denominator("
2. In both cases, one is a function and the remainder are methods of classes. Of the latter, all-bar-two are properties (that is, their only argument is self)
3. There are 482 uses of .numerator() and 650 uses of .denominator()
4. Of these 31 and 31 are calls of the non-properties methods.

This suggest that 52 of these methods could have the @property decorators added relatively simply, and 4 exception methods will require some thought / care*. Similarly 1070 uses of these methods could be replaced via a direct substitution and 62 uses will require some care. So it appears that the effort required to implement within the Sage source, while large, would be relatively low as the majority of the work could be done with a find-and-replace.

*The exceptional methods are:

```./src/sage/rings/continued_fraction.py:    def denominator(self, n):
./src/sage/symbolic/expression.pyx:    def denominator(self, bint normalize=True):
```

While the exceptional function that does not need to be changed is:

```./src/sage/misc/functional.py:def denominator(x):
```

comment:17 Changed 3 years ago by mcbell

So having attempted the solution I proposed in comment 16 (via a small sed script) I do not think that this option will work. This is due to some of the .numerator() calls being calls on non-sage classes. For example, in rings/number_field/number_field_ideal.py the line

```        return ZZ(q.numerator())
```

cannot be substitute since here q is a pari object and does not have a numer method.

comment:18 Changed 3 years ago by git

• Commit changed from 6756542a807a5fc00252e4564ea4a550d67a1a45 to 70b1ecda035696998a057f381136af6b3a3f134a

Branch pushed to git repo; I updated commit sha1. New commits:

 ​81b0862 `Attempting to make sure that callable things are not integers everywhere` ​9adc341 `Typo.` ​366ce6c `Comments to explain about the deprecation warning that could be added.` ​70b1ecd `Also dealing with hasattr(x, '__call__').`

comment:19 follow-ups: ↓ 26 ↓ 29 Changed 3 years ago by mcbell

The above commits deals with the issues raised in comment 9. It does this by

1. removing the deprecation warning - this could be added in in a later ticket and updating the relevant doctests to reflect this.
2. Replacing all uses of `hasattr(x, '__call__')` with `callable(x)`
3. Then replacing all uses of `callable(x)` with `callable(x) and not isinstance(x, numbers.Integral)`

This now means that the doctests pass and that doing

```sage: from fractions import Fraction
sage: Fraction(3, 7)
Fraction(3, 7)
sage: Fraction(3/7)
Fraction(3, 7)
```

works.

Last edited 3 years ago by mcbell (previous) (diff)

comment:20 Changed 3 years ago by mcbell

• Authors set to Mark Bell
• Status changed from new to needs_review

comment:21 Changed 3 years ago by git

• Commit changed from 70b1ecda035696998a057f381136af6b3a3f134a to 8f50595bd84c12bde0f35d0dd2529b49e62a072b

Branch pushed to git repo; I updated commit sha1. New commits:

 ​8f50595 `Also dealing with isinstance(x, collections.Callable).`

comment:22 Changed 3 years ago by git

• Commit changed from 8f50595bd84c12bde0f35d0dd2529b49e62a072b to 4977867e55d76c29b7286e2d542d8342889d8305

Branch pushed to git repo; I updated commit sha1. New commits:

 ​4977867 `Fixed tab character.`

comment:25 Changed 3 years ago by jdemeyer

To the OP: why do you care about Python fractions? It's a serious question since Guido van Rossum seems quite negative about them (see the link at 23).

comment:26 in reply to: ↑ 19 ; follow-up: ↓ 67 Changed 3 years ago by jdemeyer

1. Replacing all uses of `hasattr(x, '__call__')` with `callable(x)`

This is independent of the issue at hand and should be done in a separate ticket.

comment:27 in reply to: ↑ 15 ; follow-up: ↓ 28 Changed 3 years ago by jdemeyer

Note that it affects Python but also `numpy`.

Please elaborate. Does numpy have fractions? I couldn't find anything like that.

comment:28 in reply to: ↑ 27 Changed 3 years ago by vdelecroix

Note that it affects Python but also `numpy`.

Please elaborate. Does numpy have fractions? I couldn't find anything like that.

I meant that they are complient with the numbers abc protocol

```>>> import numpy as np
>>> np.int16(123).numerator
123
>>> np.int16(123).denominator
1
```

comment:29 in reply to: ↑ 19 ; follow-up: ↓ 32 Changed 3 years ago by jdemeyer

1. Then replacing all uses of `callable(x)` with `callable(x) and not isinstance(x, numbers.Integral)`

I really don't like this change because it's an ugly hack and it may be bad for performance (since `numbers.Integral` is an abstract base class that arbitrary classes can register for, this is quite slow). Are those changes really needed?

comment:30 follow-up: ↓ 33 Changed 3 years ago by jdemeyer

This is silly:

```    def __call__(self, *args, **kwargs):
if args or kwargs:
raise TypeError("'{}.{}' object is not callable".format(self.__class__.__module__, self.__class__.__name__))
```

If you don't want to support `*args` or `**kwargs`, just write

```    def __call__(self):
return self
```

which is more efficient.

I also disagree with

```        # If in the future we decide to deprecate this functionality then we can add:
#   deprecation(28234, 'callable integers are only intended as a tempory patch until .numerator and .denominator properties are used everywhere')
# This will require updating a large number of doctests to reflect this warning so we leave this for a future ticket.
```

I would rather keep the method as normal usage instead of deprecating it.

comment:31 Changed 3 years ago by jdemeyer

For the record, I'm not saying yet that I agree with this ticket. I'm just giving comments in case that we decide to go with this approach.

I certainly want to wait a bit to see where the Python discussion goes. It would help to have a convincing use case, instead of "I want to use `fractions.Fraction` in Sage just because".

comment:32 in reply to: ↑ 29 Changed 3 years ago by mcbell

1. Then replacing all uses of `callable(x)` with `callable(x) and not isinstance(x, numbers.Integral)`

I really don't like this change because it's an ugly hack and it may be bad for performance (since `numbers.Integral` is an abstract base class that arbitrary classes can register for, this is quite slow). Are those changes really needed?

I agree that this is ugly and that it may impact performance. However it is necessary in at least some (in fact I believe a large percentage) of the `callable(x)` cases (for example, see ​https://github.com/sagemath/sage/blob/master/src/sage/rings/polynomial/infinite_polynomial_element.py#L1523). Without these the route through the code changes and so if they are omitted then the code raises an error. The tricky thing here is that (like in the above example) the error is often raised far away from where the callable(x) occurred, making these hard to track down.

It may be possible to remove some of these uses when you explicitly know that `x` is never a Sage Integer. However I would be extremely worried about these cases since if you are wrong then it is possible that the code does not raise an error but may compute a incorrect result. Now hopefully there would be doctests that would run all of these different possibilities but this may be a situation that people aren't expecting and so I'm not sure that we can rely on the doctests to save us.

Therefore I would strongly argue that these changes are needed everywhere. In fact, even in the few cases where it is completely obvious that `x` cannot be a Sage Integer, it may be better to keep these purely for consistency.

comment:33 in reply to: ↑ 30 Changed 3 years ago by mcbell

If you don't want to support `*args` or `**kwargs`, just write

```    def __call__(self):
return self
```

which is more efficient.

I agree that this is a better way to go initially. Thank you for pointing this out.

comment:34 follow-up: ↓ 40 Changed 3 years ago by jdemeyer

The fact that making Sage integers callable breaks stuff is bad. So we should consider the pros and cons of this approach. I'm not sufficiently convinced about the pros (as Guido van Rossum said: what real world problems would this solve?). Note that the submitter at #20861 admitted to not really caring about fractions, so for the moment my feeling goes towards rejecting this fix.

comment:35 Changed 3 years ago by embray

I do find it a shame, in general, that there are so many methods on classes in Sage that should really be properties. I understand the reason ("what if you do want to add some optional arguments to that property later?"). But in that case I think there should be a separate method from the argument-less property.

It's also an unfortunate shortcoming in Python that there's know way to easily know ahead of time that when an attribute is looked up on an object, it will be subsequently called as in method call syntax. So there is no opportunity to return a different type for attribute access when doing something like `a.property` versus `a.property()`. It's fundamentally baked into Python's design that these cases can't be distinguished in some way.

I've thought in the past of working around this limitation by coming up with a generic way of proxying arbitrary types in such a way as to make them callable. So rather than making Integer callable as this ticket proposes, something like `Rational.numerator` would instead return an Integer instance wrapped in a proxy, which when called does something special with that instance, likely with reference to the original Rational that returned it (in this case the call method on the proxy would be trivial--it would just return the Integer itself).

Unfortunately that's a whole other can of worms itself, especially when you want to add a callable-proxy to types that are already callable in the first place.

comment:36 follow-up: ↓ 37 Changed 3 years ago by embray

Though I did just see Jeroen's point here that technically one could implement specialized method call syntax. But it's still really confusing. For example you'd have `a.foo()` do something different from `x = a.foo; x()`. And yes, it would be very CPython specific--but maybe that's okay, for special cases in Sage, to take advantage of?

Another super hacky workaround I've considered in the past for this (which would only work in CPython with pure Python code) would be to have a special kind of descriptor that actually looks ahead in the bytecode to see if (within some reasonable localization) the object returned by the descriptor will be called, and thus allow different behavior in that case. Again, a hugely problematic hack, but possibly "good enough" for some common cases?

comment:37 in reply to: ↑ 36 Changed 3 years ago by jdemeyer

Though I did just see Jeroen's point here that technically one could implement specialized method call syntax. But it's still really confusing. For example you'd have `a.foo()` do something different from `x = a.foo; x()`.

To expand on this, this is part of the work on PEP 590. Note the different bytecode generated:

```>>> from dis import dis
>>> def f(obj):
...     return obj.meth()
>>> def g(obj):
...     x = obj.meth
...     return x()
>>> dis(f)
4 CALL_METHOD              0
6 RETURN_VALUE
>>> dis(g)
4 STORE_FAST               1 (x)

8 CALL_FUNCTION            0
10 RETURN_VALUE
```

`LOAD_METHOD` and `CALL_METHOD` are an optimization to avoid creating a temporary bound method object when executing `obj.meth()`. It basically turns the call into a call of the unbound method. This optimization used to be enabled only for two specific method classes (one for Python methods and one for C methods). But PEP 590 allows this optimization for arbitrary classes. In theory, it should just be an optimization not affecting the actual behaviour. But it does allow doing arbitrary hacky stuff when `LOAD_METHOD`/`CALL_METHOD` is used.

And yes, it would be very CPython specific--but maybe that's okay, for special cases in Sage, to take advantage of?

We need to make sure that Cython supports it too. Since PEP 590 is so new, it's not in the current stable release. But the big upcoming Cython 3.0 should at least partially support it.

comment:38 Changed 3 years ago by jdemeyer

I forgot to mention that PEP 590 needs Python 3.8, for which there is no stable release yet.

comment:39 follow-up: ↓ 41 Changed 3 years ago by embray

How does it know to generate special bytecode in this case? Is it just hacked into the parser somehow?

Part of me really does think there should be a way to exploit direct method calls as a special case, even though it would be a slightly confusing inconsistency.

comment:40 in reply to: ↑ 34 ; follow-up: ↓ 42 Changed 3 years ago by slelievre

as Guido van Rossum said: what real world problems would this solve?

Some mathematical packages for dynamics, geometry and topology of surfaces and 3-manifolds, such as Curver, Flipper, SnapPy, Twister, Veerer, can be run either with pure Python, or with Sage. Some of them use the `Fraction` constructor from Python's `fractions` module, which works with Python ints or rationals and fails with Sage integers or rationals. That's the real-world problem.

comment:41 in reply to: ↑ 39 Changed 3 years ago by jdemeyer

Is it just hacked into the parser somehow?

Yes. When the parser sees something of the form `obj.attr()`, it uses those special bytecodes. Further details in https://bugs.python.org/issue26110

comment:42 in reply to: ↑ 40 ; follow-up: ↓ 46 Changed 3 years ago by jdemeyer

Some mathematical packages for dynamics, geometry and topology of surfaces and 3-manifolds, such as Curver, Flipper, SnapPy, Twister, Veerer, can be run either with pure Python, or with Sage. Some of them use the `Fraction` constructor from Python's `fractions` module

OK, I quickly looked at these packages. First of all, I couldn't find a package called "Veerer".

Two don't use fractions:

Two do use fractions:

However, it's not obvious to me how Sage integers or rationals could end up as an argument to `Fraction` in these packages. curver doesn't use Sage at all. snappy does use Sage (for example, for Gröbner basis computations), but it seems that output from Sage is converted to snappy data structures, so it doesn't really do computations with Sage data types.

comment:43 follow-up: ↓ 44 Changed 3 years ago by vdelecroix

`flipper` does indirectly uses `Fraction` through realalg. And it is easy to break

```sage: import realalg
sage: realalg.RealNumberField([1,0,-2])
Traceback (most recent call last):
...
TypeError: invalid input: <built-in method numerator of sage.rings.integer.Integer object at 0x7fca4682fd20>
```

I thought about providing support for sage `Rational` in `realalg`... but it is a can of worms.

comment:44 in reply to: ↑ 43 ; follow-up: ↓ 45 Changed 3 years ago by jdemeyer

but it is a can of worms.

A can of worms is precisely what we're looking for here. Guido claims that there is no problem, so we some data to prove him wrong.

comment:45 in reply to: ↑ 44 Changed 3 years ago by vdelecroix

but it is a can of worms.

A can of worms is precisely what we're looking for here. Guido claims that there is no problem, so we some data to prove him wrong.

Agreed.

As we make the PEP idea better, we should also think about making `Rational` an easy drop-in replacement for `Fraction`. In other words, the following kind of thing should work

```try:
from sage.rings.rational import Rational as Fraction
except ImportError:
try:
from gmpy2 import mpq as Fraction
except ImportError:
from fractions import Fraction
```

I see a serious obstruction for now: the constructors are not compatible.

```sage: from fractions import Fraction
sage: from gmpy2 import mpq
sage: Fraction(2r, 3r)
Fraction(2, 3)
sage: mpq(2,3)
mpq(2,3)
sage: Rational(2,3)   # second argument is "base"
2
```

comment:46 in reply to: ↑ 42 Changed 3 years ago by mcbell

Two do use fractions:

However, it's not obvious to me how Sage integers or rationals could end up as an argument to `Fraction` in these packages. curver doesn't use Sage at all.

Sage Integers can end up in Fractions whenever a user uses these packages interactively or from within a sage script / notebook. For example:

```sage: import curver
sage: curver.load(2, 1).triangulation([0, 99, 99, 1, 100, 0, 0, 0, 0])
```

crashes since the values that are passed to the functions within curver are sage Integers. This happens even though curver has scattered throughout it lines (equivalent to):

```    assert isinstance(n, numbers.Integral)
```

but since Sage Integers have been registered as Integral these catches do not work.

Of course, doing

```sage: curver.load(2, 1).triangulation([0r, 99r, 99r, 1r, 100r, 0r, 0r, 0r, 0r])
```

works without an issue.

comment:47 Changed 3 years ago by mcbell

Personally, I am more worried about code that does incorrect / dangerous things when it encounters. For example, I could imagine something like:

```def prod_numerator_denominator(x):
''' Since x = p / q (where q is possibly 1), return p * q. '''
try:
# If x is Rational then this will work.
return x.numerator * x.denominator
except:
# x was not a Rational, so we can take q == 1 and just return x
return x

assert prod_numerator_denominator(Fraction(3r, 2r)) == 6
assert prod_numerator_denominator(3) == 3
assert prod_numerator_denominator(3 / 2) == 6  # Fails since prod_numerator_denominator(3 / 2) returns 3
```

Or even

```def f(x):
if not isinstance(x, numbers.Rational):
return NotImplemented

# So now I know x is a Rational and so implements .numerator and .denominator properties
try:
return x.numerator + x.denominator
except:
# I know the code here can never be run so I can it should be safe to put something bad here
import os
os.remove('/')
```
Last edited 3 years ago by mcbell (previous) (diff)

comment:48 Changed 3 years ago by embray

To me an obvious answer here is for Sage `Rational`s and `Integer`s to just get some kind of `.as_pyfraction()` method to convert them to a `Fraction`.

It's not ideal of course. One would really prefer to see it just work, transparently. But when interfaces collide the only recourse, if you can't easily change the interfaces, is to make some kind of adapter.

I liked Jeroen's `__ratio__` proposal and thought it made a lot of sense. But it seems it wasn't too popular. And even if it did happen one day we'd have to wait for the change in Python (or monkey-patch the `fractions` module which I suppose is another possibility...)

comment:49 follow-up: ↓ 50 Changed 3 years ago by slelievre

I often wished there was `a.num_denom()` to get `a.numerator()` and `a.denominator()` at once, the same way `a.quo_rem(b)` gives the quotient `a // b` and the remainder `a % b` at once.

comment:50 in reply to: ↑ 49 Changed 3 years ago by jdemeyer

I often wished there was `a.num_denom()`

It's called `.as_integer_ratio()` and it will be supported in Python 3.8 for all standard numeric types (`int`, `float`, `decimal.Decimal` and `fractions.Fraction`). Clearly, this is something that Sage types should also have.

This doesn't yet solve the problem that this ticket wants to solve, but it's getting closer.

comment:51 Changed 3 years ago by slelievre

Okay, I opened #28348 for that.

comment:52 Changed 3 years ago by jdemeyer

I opened #28347 for that, needs review.

comment:53 Changed 2 years ago by embray

• Milestone changed from sage-8.9 to sage-9.1

Ticket retargeted after milestone closed

comment:54 Changed 2 years ago by mkoeppe

• Milestone changed from sage-9.1 to sage-9.2

Batch modifying tickets that will likely not be ready for 9.1, based on a review of the ticket title, branch/review status, and last modification date.

comment:55 Changed 22 months ago by mkoeppe

• Status changed from needs_review to needs_work

merge conflict

comment:56 Changed 21 months ago by mkoeppe

• Milestone changed from sage-9.2 to sage-9.3

comment:57 Changed 16 months ago by embray

Any interest in continuing to work on this? I've just been bitten by this issue in my own code.

comment:58 Changed 15 months ago by mkoeppe

• Milestone changed from sage-9.3 to sage-9.4

Setting new milestone based on a cursory review of ticket status, priority, and last modification date.

comment:59 Changed 14 months ago by slelievre

• Description modified (diff)

This issue was already reported in #10928, comment 14 (a ticket about getitem of numpy matrix using Sage integers).

Hopefully with #28347 in, things are easier than back at Sage Days 100?

comment:60 Changed 10 months ago by mkoeppe

• Milestone changed from sage-9.4 to sage-9.5

Setting a new milestone for this ticket based on a cursory review.

comment:62 Changed 9 months ago by mkoeppe

This issue also shows up when trying to use the `statistics` module with Sage integers or rationals, see #29662.

Although the `as_integer_ratio` mechanism has been added to Python, statistics._exact_ratio does not use `as_integer_ratio` because it sees the Sage numbers' `numerator` and `denominator` methods and tries to use them as properties.

comment:63 Changed 9 months ago by mkoeppe

• Summary changed from Sage Integers are incompatible with Python Fractions to Sage Integers are incompatible with Python Fractions and statistics.mean etc.

comment:64 Changed 5 months ago by mkoeppe

• Description modified (diff)
• Summary changed from Sage Integers are incompatible with Python Fractions and statistics.mean etc. to Sage Integers are incompatible with Python fraction.Fraction, statistics.mean, itertools.permutations

comment:65 Changed 5 months ago by mkoeppe

• Description modified (diff)

comment:66 Changed 5 months ago by mkoeppe

• Milestone changed from sage-9.5 to sage-9.6

comment:67 in reply to: ↑ 26 Changed 3 months ago by jhpalmieri

1. Replacing all uses of `hasattr(x, '__call__')` with `callable(x)`