Opened 3 years ago

Closed 2 years ago

#28394 closed defect (fixed)

comparison of sage rationals with gmpy2 mpq broken

Reported by: Vincent Delecroix Owned by:
Priority: major Milestone: sage-9.2
Component: basic arithmetic Keywords:
Cc: vklein Merged in:
Authors: Vincent Delecroix Reviewers: Matthias Koeppe
Report Upstream: Fixed upstream, in a later stable release. Work issues:
Branch: 21d07f6 (Commits, GitHub, GitLab) Commit: 21d07f652037874491cfa08dc569d10a30285084
Dependencies: #30583 Stopgaps:

Status badges

Description (last modified by vklein)

sage: import gmpy2
sage: gmpy2.mpq(5,3) == 5/3
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-2-7445d1641afe> in <module>()
----> 1 gmpy2.mpq(Integer(5),Integer(3)) == Integer(5)/Integer(3)

TypeError: cannot convert object to mpq
sage: 5/3 == gmpy2.mpq(5,3)
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-3-593205bfa17f> in <module>()
----> 1 Integer(5)/Integer(3) == gmpy2.mpq(Integer(5),Integer(3))

/opt/sage/sage-py3-gcc/local/lib/python3.7/site-packages/sage/rings/rational.pyx in sage.rings.rational.Rational.__richcmp__ (build/cythonized/sage/rings/rational.c:9498)()
    868             c = mpq_cmp_si((<Rational>left).value, PyInt_AS_LONG(right), 1)
    869         else:
--> 870             return coercion_model.richcmp(left, right, op)
    871 
    872         return rich_to_bool_sgn(op, c)

/opt/sage/sage-py3-gcc/local/lib/python3.7/site-packages/sage/structure/coerce.pyx in sage.structure.coerce.CoercionModel.richcmp (build/cythonized/sage/structure/coerce.c:19779)()
   1946         # we would end up trying the same coercion again.
   1947         if not y_is_Element and Py_TYPE(y).tp_richcompare:
-> 1948             res = Py_TYPE(y).tp_richcompare(y, x, revop(op))
   1949             if res is not NotImplemented:
   1950                 return res

TypeError: cannot convert object to mpq

Sage integers are perfectly fine

sage: gmpy2.mpz(3) == 3
True
sage: 3 == gmpy2.mpz(3)
True

gmpy2's issue #251

Change History (14)

comment:1 Changed 3 years ago by vklein

It looks likes it's a gmpy2 bug.

python3.7
Python 3.7.4 (default, Jul  9 2019, 03:52:42) 
[GCC 5.4.0 20160609] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from gmpy2 import mpz, mpq
>>> class Q:
...     def __mpq__(self): return mpq(3,2)
... 
>>> q = Q()
>>> mpq(3, 2) * q
mpq(9,4)
>>> mpq(3, 2) == q
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: cannot convert object to mpq
>>> from gmpy2 import cmp
>>> cmp(mpq(3,2), q)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: cannot convert object to mpq
>>> cmp(mpq(3,2), mpq(3,2))
0

comment:2 Changed 3 years ago by vklein

Description: modified (diff)
Report Upstream: N/AReported upstream. No feedback yet.

comment:3 Changed 3 years ago by vklein

Report Upstream: Reported upstream. No feedback yet.Fixed upstream, in a later stable release.

comment:4 Changed 3 years ago by vklein

Do you think this need a patch ?

IMHO it's not necessary. You can call __mpq__ explicitly if needed :

sage: from gmpy2 import mpq
sage: q = 5/3
sage: q.__mpq__() == mpq(5,3)
True
sage: mpq(q) == mpq(5,3)
True

comment:5 Changed 3 years ago by Vincent Delecroix

Thank you. I agree with you that there is no need for a patch.

comment:6 Changed 3 years ago by Erik Bray

Milestone: sage-8.9sage-9.1

Ticket retargeted after milestone closed

comment:7 Changed 2 years ago by Matthias Köppe

Milestone: sage-9.1sage-9.2

Moving tickets to milestone sage-9.2 based on a review of last modification date, branch status, and severity.

comment:8 Changed 2 years ago by Samuel Lelièvre

Why TypeError: cannot convert object to mpq if converting is as simple as mpq(5/3)?

comment:9 Changed 2 years ago by Vincent Delecroix

This has been fixed upstream

sage: gmpy2.mpq(5,3) == 5/3
True
sage: 5/3 == gmpy2.mpq(5,3)
True

EDIT: in version 2.1.0b4

Last edited 2 years ago by Vincent Delecroix (previous) (diff)

comment:10 Changed 2 years ago by Vincent Delecroix

Dependencies: #30583

comment:11 Changed 2 years ago by Vincent Delecroix

Authors: Vincent Delecroix
Branch: u/vdelecroix/28394
Commit: 0e5d1f3f9992bd9396eda3e79b479a1e80140bcd
Status: newneeds_review

New commits:

6ea912730583: upgrade gmpy2 to 2.1.0b5
0e5d1f3test comparisons between Sage rationals and gmpy2 mpq

comment:12 Changed 2 years ago by Matthias Köppe

Branch: u/vdelecroix/28394u/mkoeppe/28394

comment:13 Changed 2 years ago by Matthias Köppe

Commit: 0e5d1f3f9992bd9396eda3e79b479a1e80140bcd21d07f652037874491cfa08dc569d10a30285084
Reviewers: Matthias Koeppe
Status: needs_reviewpositive_review

New commits:

3d48632build/pkgs/gmpy2/checksums.ini: Add upstream_url
c8b0584Merge tag '9.2.beta12' into t/30583/30583
5723009build/pkgs/sagelib/src/requirements.txt: Update gmpy2 version
21d07f6Merge branch 't/30583/30583' into t/28394/28394

comment:14 Changed 2 years ago by Volker Braun

Branch: u/mkoeppe/2839421d07f652037874491cfa08dc569d10a30285084
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.