Opened 2 years ago

Last modified 18 months ago

#22928 closed enhancement

Conversion between gmpy2 and sage integers — at Version 14

Reported by: vklein Owned by: vklein
Priority: major Milestone: sage-8.2
Component: interfaces Keywords: thursdaysbdx
Cc: vdelecroix Merged in:
Authors: Vincent Klein Reviewers:
Report Upstream: N/A Work issues:
Branch: u/vklein/conversion_between_gmpy2_and_sage_integers (Commits) Commit: 54c04ae7c0bf7d48ca89c1e21c4184133e4c6a9c
Dependencies: #22927 Stopgaps:

Description (last modified by vdelecroix)

Conversion between gmpy2 and sage integers:

  • Implement method __mpz__ on sage integer: return an gmpy2 mpz
  • Implement method __mpq__ on sage rational: return an gmpy2 mpq
  • Add conversion functions from gmpy2's mpz and mpq to sage integer and rational.
  • Provide coercion.

Once gmpy2 is a standard package with the following features :

  • direct constructors GMPy_MPZ_From_mpz, GMPy_MPQ_From_mpq, GMPy_MPQ_From_mpz
  • #137 a gmpy2.pxd file with relevant declarations (to be installed at the same place as gmpy2.h)
  • #136 robust header detection (ie what should the user do in her setup.py to find out where gmpy2.h/gmpy2.pxd are?)

see : #134, #136, #137.

Then remove sage.libs.gmpy2 extension and replace conversion functions gmpy2_mpz_to_sage and gmpy2_mpq_to_sage by having sage integer and rational constructors working with gmpy2 number

Change History (14)

comment:1 Changed 2 years ago by vklein

  • Dependencies set to #22927

comment:2 Changed 2 years ago by vklein

  • Owner changed from (none) to vklein

comment:3 Changed 2 years ago by vdelecroix

  • Keywords jsb++ added

comment:4 Changed 2 years ago by vdelecroix

  • Keywords thursdaysbdx added; jsb++ removed

comment:5 Changed 2 years ago by vklein

  • Description modified (diff)

comment:6 Changed 2 years ago by vklein

  • Branch set to u/vklein/conversion_between_gmpy2_and_sage_integers

comment:7 Changed 2 years ago by git

  • Commit set to 48daf5857197f66b880a563ce27c34cf21959c87

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

48daf58Add path of gmpy2 in include_dirs

comment:8 follow-up: Changed 2 years ago by vdelecroix

Looks good. Some small remarks.

1) For the error about missing gmpy2 you can use PackageNotFoundError from sage.misc.package

2) add __mpz__() to class Rational should be add __mpq__() to class Rational

3) You have an indentation problem here

Coercion for gmpy2 numbers    
    sage: from gmpy2 import * # optional - gmpy2
    sage: a, b = cm.canonical_coercion(mpz(5), 10) # optional - gmpy2
    sage: a, b # optional - gmpy2

It should be

Coercion for gmpy2 numbers::

    sage: from gmpy2 import * # optional - gmpy2
    sage: a, b = cm.canonical_coercion(mpz(5), 10) # optional - gmpy2
    sage: a, b # optional - gmpy2

4) There should be tests for a + b, a * b, a / b when one of a or b is from Sage and the other from gmpy2

5) Couldn't you share

#Handle gmpy2 objects
elif is_gmpy2_type(type(x)):
    return self.canonical_coercion(py_scalar_to_element(x), y)
elif is_gmpy2_type(type(y)):
    return self.canonical_coercion(x, py_scalar_to_element(y))

with the associated code that deals with numpy types?

Last edited 2 years ago by vdelecroix (previous) (diff)

comment:9 Changed 2 years ago by git

  • Commit changed from 48daf5857197f66b880a563ce27c34cf21959c87 to 54c04ae7c0bf7d48ca89c1e21c4184133e4c6a9c

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

54c04aeMinor improvements

comment:10 in reply to: ↑ 8 Changed 2 years ago by vklein

Replying to vdelecroix: Done with commit 54c04ae

Last edited 2 years ago by vklein (previous) (diff)

comment:11 Changed 2 years ago by vklein

  • Description modified (diff)

comment:12 Changed 2 years ago by vklein

  • Description modified (diff)

comment:13 Changed 2 years ago by vklein

  • Description modified (diff)

comment:14 Changed 2 years ago by vdelecroix

  • Description modified (diff)
Note: See TracTickets for help on using tickets.