-
Notifications
You must be signed in to change notification settings - Fork 33
Description
Currently it is a bit awkward to use flint's fmpz and fmpq because they do not interoperate with anything else:
In [1]: import flint
In [2]: import gmpy2
In [3]: flint.fmpz(2) * gmpy2.mpz(2)
...
TypeError: unsupported operand type(s) for *: 'flint._flint.fmpz' and 'mpz'
I'm not sure that coercion in binary operations really makes sense because e.g. in the above case it is not clear whether the result should be returned as fmpz
or mpz
. However at least explicit conversions would be nice:
In [4]: flint.fmpz(gmpy2.mpz(2))
...
TypeError: cannot create fmpz from type <class 'mpz'>
In [5]: gmpy2.mpz(flint.fmpz(2))
Out[5]: mpz(2)
The gmpy2 conversion above works (I think) because it uses __index__
which is defined for fmpz
. A basic improvement would be for fmpz.__new__
to use __index__
when converting non-int types and then conversion from mpz
would at least work.
Using __index__
means going via CPython's int
which is inefficient because there is a double conversion and a different limb size. It would be nice if there was a more direct way to convert between mpz
and fmpz
but I am not sure how easy that is to do if there is any possibility that limb sizes might be set differently at build time.
Converting to from Python int is slow. Currently the conversion goes via a hex Python string:
python-flint/src/flint/fmpz.pyx
Lines 1 to 31 in 99fa557
cdef inline int fmpz_set_pylong(fmpz_t x, obj): | |
cdef int overflow | |
cdef slong longval | |
longval = pylong_as_slong(<PyObject*>obj, &overflow) | |
if overflow: | |
s = "%x" % obj | |
fmpz_set_str(x, chars_from_str(s), 16) | |
else: | |
fmpz_set_si(x, longval) | |
cdef inline int fmpz_set_python(fmpz_t x, obj): | |
if PY_MAJOR_VERSION < 3 and PyInt_Check(<PyObject*>obj): | |
fmpz_set_si(x, PyInt_AS_LONG(<PyObject*>obj)) | |
return 1 | |
if PyLong_Check(<PyObject*>obj): | |
fmpz_set_pylong(x, obj) | |
return 1 | |
return 0 | |
cdef fmpz_get_intlong(fmpz_t x): | |
""" | |
Convert fmpz_t to a Python int or long. | |
""" | |
cdef char * s | |
if COEFF_IS_MPZ(x[0]): | |
s = fmpz_get_str(NULL, 16, x) | |
v = int(str_from_chars(s), 16) | |
libc.stdlib.free(s) | |
return v | |
else: | |
return <slong>x[0] |
Ideally there would be some code that could convert more directly between the two different representations with their different limb sizes although I am not sure if CPython provides any "public" API that could be used for this.
Conversion to float works for fmpz but not fmpq:
In [8]: float(flint.fmpz(2))
Out[8]: 2.0
In [9]: float(flint.fmpq(2))
---------------------------------------------------------------------------
TypeError
This could be fixed by defining fmpq.__float__
.
Converting from float does not work:
In [11]: flint.fmpz(2.0)
---------------------------------------------------------------------------
TypeError
In [12]: flint.fmpq(2.0)
---------------------------------------------------------------------------
ValueError
I think that it would be nice to enable this for fmpq
but I am not sure about fmpz
.
Possibly the stdlib fractions module should be able to coerce:
In [13]: import fractions
In [14]: fractions.Fraction(1, 2) * flint.fmpz(2)
---------------------------------------------------------------------------
TypeError
At least I think that the numeric tower defines numerator and denominator so this could be made to work by checking those:
In [16]: q = fractions.Fraction(2, 3)
In [17]: flint.fmpq(q) # the error message could be improved
...
ValueError: cannot create fmpq from object of type <class 'NotImplementedType'>
In [18]: flint.fmpq(q.numerator, q.denominator)
Out[18]: 2/3
Interaction with mpmath also does not work. I wonder though if it makes more sense to support that on the mpmath side:
In [19]: import mpmath
In [21]: mpmath.mpf(2) * flint.fmpz(2)
---------------------------------------------------------------------------
TypeError
Perhaps mpmath should have the option to use python-flint for ground types and should add support for coercing python-flint types where it makes sense.