Opened 4 years ago

Closed 2 years ago

Last modified 2 years ago

#20238 closed task (fixed)

Move the Sage <-> PARI interface to a stand-alone package CyPari

Reported by: jdemeyer Owned by:
Priority: major Milestone: sage-8.0
Component: packages: standard Keywords: Pari
Cc: defeo, slelievre, vdelecroix, mmarco, jpflori Merged in:
Authors: Jeroen Demeyer Reviewers: Jean-Pierre Flori
Report Upstream: N/A Work issues:
Branch: a56fffe (Commits) Commit:
Dependencies: #22728 Stopgaps:

Description (last modified by jdemeyer)

Needs cysignals:

  • #20002: Move interrupt.pyx to package cysignals
  • #20210: Move memory functions to cysignals

Cleanup of PARI interface:

  • #20205: Clean up factoring PARI interface
  • #20213: Replace pari_catch_sig_on by sig_on
  • #20216: Deprecate PARI nth_prime, prime_list, primes_up_to_n
  • #20217: Remove redundant functions from pari_instance.pyx
  • #20219: Remove redundant functions from gen.pyx
  • #20224: Auto-generated PARI functions sometimes return 0 instead of None
  • #20226: Implement conversion PARI <-> Python int/long without GMP/MPIR
  • #20241: Separate Sage-specific components from generic C-interface in PariInstance
  • #20257: Deprecate undocumented arguments to PARI functions
  • #20352: Initialize PARI constants in PariInstance.__init__
  • #20473: Remove global pari_instance variable
  • #20486: Remove deprecated PARI code
  • #21703: Interface PARI precision in bits
  • #21806: Allow multiple instances of the PariInstance object
  • #21807: Move gentoobj and rename it to gentosage
  • #21808: Change gen.python() to return Python objects
  • #21809: Pythonize deprecation warnings in PARI interface
  • #21810: Move calculation of PARI stack size out of __init__
  • #22183: Rename PariInstance -> Pari
  • #22185: Rename gen -> Gen
  • #22195: Allow compiling PARI interface without anal.h
  • #22165: Stop using deprecated PARI function polred()
  • #22210: Remove obsolete special case in PARI Gen.eval()
  • #22221: Fix dependency on PARI headers
  • #22222: Remove pari_instance global in gen.pyx
  • #22319: Implement __index__ for PARI Gen
  • #22471: Gen: clean up "new_ref" mechanism
  • #22561: Implement __iter__ for PARI Gen
  • #22584: Debug build doctests fail with sig_block() with sig_on_count = 1, block_sigint = 1

PARI upgrades:

  • #20581: Upgrade PARI to latest master
  • #21005: Update PARI to version 2.8.0
  • #21756: Update PARI to version 2.9.1
  • #22276: Fix PARI patches
  • #22675: Upgrade PARI to version 2.9.2

Decouple CyPari from the Sage coercion model:

  • #20740: Drop return type from arithmetic methods in coercion model
  • #20761: Drop argument types from arithmetic methods in coercion model
  • #20757: Drop argument types from comparison methods in coercion model
  • #20686: Refactor getattr_from_other_class() for lookup of methods in categories
  • #20767: Move coercion to Element
  • #21163: In richcmp, fall back to reversed operation if coercion fails
  • #21158: Decouple PARI from coercion model

Move the PARI interface to a new package:

  • #21820: Split src/sage/libs/pari
  • #21821: Avoid Sage-specific variables in autogen/pari
  • #21874: Make autogen/pari Python 3 compatible
  • #22193: Move Sage doctests into standalone tests in src/tests
  • #22198: Make cypari2 Python2 / Python3 compatible
  • #22239: Don't use six in sage_setup/autogen/pari
  • #22240: Use relative imports in sage_setup/autogen/pari
  • #22291: ob_size does not exist in Python 3
  • #22470: Replace _pari_ -> __pari__
  • #22728: Patch Cython to find includes better

To be fixed in CyPari2:

  • #22321: Gen.__init__() does not work as expected

Tarball: http://sage.ugent.be/www/jdemeyer/sage/cypari2-1.0.0.tar.bz2 (see https://github.com/defeo/cypari2/pull/11)

Change History (72)

comment:1 Changed 4 years ago by jdemeyer

  • Summary changed from Make the Sage <-> PARI interface a stand-alone package to Move the Sage <-> PARI interface to a stand-alone package CyPari

comment:2 Changed 4 years ago by defeo

  • Description modified (diff)

comment:3 Changed 4 years ago by slelievre

  • Cc slelievre vdelecroix added
  • Keywords Pari added

comment:4 Changed 4 years ago by jdemeyer

  • Description modified (diff)

comment:5 Changed 4 years ago by jdemeyer

  • Description modified (diff)

comment:6 Changed 4 years ago by jdemeyer

  • Description modified (diff)

comment:7 Changed 4 years ago by slelievre

  • Description modified (diff)

comment:8 Changed 4 years ago by defeo

A comment by kedlaya on #20241:

Regarding the utility functions: I think primes belongs in sage.rings.fast_arith along with prime_range, while genus2red should be in sage.schemes.hyperelliptic_curves.HyperellipticCurve_g2_rational_field which is otherwise completely empty! The other functions don't provide any new functionality (factorials, Chebyshev polynomials, etc.).

comment:9 Changed 3 years ago by jdemeyer

  • Description modified (diff)

comment:10 Changed 3 years ago by jdemeyer

  • Description modified (diff)

comment:11 Changed 3 years ago by jdemeyer

  • Description modified (diff)

comment:12 Changed 3 years ago by jdemeyer

  • Description modified (diff)

comment:13 Changed 3 years ago by nbruin

Just a question: presently we have efficient sage.Integer <-> pari-gp integer conversion, right? CyPari would probably not support this directly, because that would make sage.Integer a dependency of the package. Where would the efficient conversion fit? (we wouldn't want to go through Python integers if we don't have to)

comment:14 Changed 3 years ago by jdemeyer

  • Description modified (diff)
  • Milestone changed from sage-7.2 to sage-7.3

comment:15 Changed 3 years ago by jdemeyer

  • Description modified (diff)

comment:16 Changed 3 years ago by jdemeyer

  • Description modified (diff)

comment:17 Changed 3 years ago by jdemeyer

  • Description modified (diff)

comment:18 Changed 3 years ago by jdemeyer

  • Description modified (diff)

comment:19 Changed 3 years ago by defeo

  • Cc mmarco added

comment:20 Changed 3 years ago by defeo

So, what's left to be done? Are we ready to externalize the interface?

comment:21 Changed 3 years ago by jdemeyer

  • Description modified (diff)

When the next beta with #20241 is released, I will have a look.

comment:22 Changed 3 years ago by jdemeyer

  • Description modified (diff)
  • Milestone changed from sage-7.3 to sage-7.5

comment:23 Changed 3 years ago by jdemeyer

  • Description modified (diff)

comment:24 Changed 3 years ago by jdemeyer

  • Description modified (diff)

comment:25 Changed 3 years ago by jdemeyer

  • Description modified (diff)

comment:26 Changed 3 years ago by jdemeyer

  • Description modified (diff)

comment:27 Changed 3 years ago by jdemeyer

  • Description modified (diff)

comment:28 Changed 3 years ago by jdemeyer

  • Description modified (diff)

comment:29 Changed 3 years ago by vdelecroix

  • Description modified (diff)

comment:30 Changed 3 years ago by jdemeyer

  • Description modified (diff)

comment:31 Changed 3 years ago by vdelecroix

  • Description modified (diff)

comment:32 Changed 3 years ago by jdemeyer

  • Description modified (diff)

comment:33 Changed 3 years ago by vdelecroix

  • Description modified (diff)

comment:34 Changed 3 years ago by vdelecroix

  • Description modified (diff)

comment:35 Changed 3 years ago by jdemeyer

  • Description modified (diff)

comment:36 Changed 3 years ago by jdemeyer

  • Description modified (diff)

comment:37 Changed 3 years ago by jdemeyer

  • Description modified (diff)

comment:38 Changed 3 years ago by jdemeyer

  • Description modified (diff)

comment:39 Changed 3 years ago by jdemeyer

  • Description modified (diff)

comment:40 Changed 3 years ago by jdemeyer

  • Description modified (diff)

comment:41 Changed 3 years ago by jdemeyer

  • Description modified (diff)

comment:42 Changed 3 years ago by jdemeyer

  • Description modified (diff)

comment:43 Changed 3 years ago by jdemeyer

  • Description modified (diff)

comment:44 Changed 3 years ago by jdemeyer

  • Description modified (diff)

comment:45 Changed 3 years ago by jdemeyer

  • Description modified (diff)

comment:46 Changed 3 years ago by jdemeyer

  • Description modified (diff)

comment:47 Changed 3 years ago by jdemeyer

  • Description modified (diff)

comment:48 Changed 3 years ago by jdemeyer

  • Description modified (diff)

comment:49 Changed 3 years ago by jdemeyer

  • Authors set to Jeroen Demeyer
  • Milestone changed from sage-7.5 to sage-8.0

comment:50 Changed 3 years ago by jdemeyer

There is one issue which comes up regularly when dealing with Cython code. When you do something like

cdef extern from "cypari.h":
    GEN set_gel(GEN x, long n, GEN z)              # gel(x, n) = z
    GEN set_gmael(GEN x, long i, long j, GEN z)    # gmael(x, i, j) = z
    GEN set_gcoeff(GEN x, long i, long j, GEN z)   # gcoeff(x, i, j) = z
    GEN set_uel(GEN x, long n, ulong z)            # uel(x, n) = z

the C compiler does not know where to find cypari.h. I had a similar issue in cysignals, for which I implemented an ugly work-around by auto-generating some __init__.pxd file (this wasn't so bad, since I needed to auto-generate that file anyway).

However, for a proper solution, Cython should add the include directory because Cython does know where to find cypari.h

comment:51 Changed 3 years ago by jdemeyer

  • Description modified (diff)

So the plan is now to add a Cython patch: #22728.

comment:52 Changed 3 years ago by jdemeyer

  • Dependencies set to #22728

comment:53 Changed 3 years ago by jdemeyer

  • Branch set to u/jdemeyer/move_the_sage_____pari_interface_to_a_stand_alone_package_cypari

comment:54 Changed 3 years ago by git

  • Commit set to 8e4e13f48dcf779dcb74e3e8f307b48876c49807

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

c3005c0Add cypari2 package
5be743bRemove cypari2 from Sage
90f3abaReplace sage.libs.cypari2 by cypari2
526cf13Remove deprecated file src/sage/libs/pari/decl.pxi
8e4e13fClean up imports

comment:55 Changed 3 years ago by git

  • Commit changed from 8e4e13f48dcf779dcb74e3e8f307b48876c49807 to 38124963163999f6972328e6a916c80a7e3bcd6b

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

3812496Clean up imports

comment:56 Changed 3 years ago by jdemeyer

  • Status changed from new to needs_review

comment:57 Changed 3 years ago by git

  • Commit changed from 38124963163999f6972328e6a916c80a7e3bcd6b to a56fffe5458202647f66f338a94a9024a0e83a88

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

5e35750Add cypari2 package
35af412Remove cypari2 from Sage
3650f16Replace sage.libs.cypari2 by cypari2
e895bc4Remove deprecated file src/sage/libs/pari/decl.pxi
a56fffeClean up imports

comment:58 Changed 3 years ago by jdemeyer

  • Description modified (diff)

comment:59 Changed 3 years ago by jdemeyer

  • Description modified (diff)

comment:60 follow-up: Changed 2 years ago by isuruf

Another solution would be to install the header into a standard location using headers option of distutils.core.setup. See https://docs.python.org/3/distutils/setupscript.html#preprocessor-options for why that works.

If you do that, you need to change cdef extern from "cypari.h": to cdef extern from "cypari2/cypari.h": because distutils will install cypari.h to cypari2/cypari.h

comment:61 in reply to: ↑ 60 Changed 2 years ago by jdemeyer

Replying to isuruf:

Another solution would be to install the header into a standard location using headers option of distutils.core.setup. See https://docs.python.org/3/distutils/setupscript.html#preprocessor-options for why that works.

Right. However, it seems that Python packages rarely use this option. I wonder if there is a reason for that...

comment:62 Changed 2 years ago by jdemeyer

Reading that page https://docs.python.org/3/distutils/setupscript.html#preprocessor-options better, it only talks about what you need to do in order to compile against the header. It doesn't say how to actually install the headers.

Here is a "interesting" post about Python header files: https://github.com/pypa/packaging-problems/issues/84

comment:63 Changed 2 years ago by jpflori

  • Cc jpflori added

comment:64 follow-up: Changed 2 years ago by jpflori

This looks pretty trivial and good. My only concern is: is it cypari or cypari2?

comment:65 in reply to: ↑ 64 Changed 2 years ago by jdemeyer

Replying to jpflori:

My only concern is: is it cypari or cypari2?

The Python package is currently called CyPari2, but the goal is to merge it with CyPari?.

comment:66 Changed 2 years ago by jpflori

  • Reviewers set to Jean-Pierre Flori
  • Status changed from needs_review to positive_review

comment:67 Changed 2 years ago by vbraun

  • Branch changed from u/jdemeyer/move_the_sage_____pari_interface_to_a_stand_alone_package_cypari to a56fffe5458202647f66f338a94a9024a0e83a88
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:68 Changed 2 years ago by jdemeyer

  • Commit a56fffe5458202647f66f338a94a9024a0e83a88 deleted

Great news! I will officially release CyPari2-1.0.0 then and put it on pypi.

comment:69 follow-ups: Changed 2 years ago by jhpalmieri

After updating to the latest beta, I have some stale autogenerated files in src/sage/lib/cypari2: the ones listed in the now deleted .gitignore file. Should it be the job of the CyPari2 spkg-install file to delete those?

comment:70 in reply to: ↑ 69 ; follow-up: Changed 2 years ago by vdelecroix

Replying to jhpalmieri:

After updating to the latest beta, I have some stale autogenerated files in src/sage/lib/cypari2: the ones listed in the now deleted .gitignore file. Should it be the job of the CyPari2 spkg-install file to delete those?

They were just not removed because not tracked by git, aren't they?

comment:71 in reply to: ↑ 70 Changed 2 years ago by jhpalmieri

Replying to vdelecroix:

Replying to jhpalmieri:

After updating to the latest beta, I have some stale autogenerated files in src/sage/lib/cypari2: the ones listed in the now deleted .gitignore file. Should it be the job of the CyPari2 spkg-install file to delete those?

They were just not removed because not tracked by git, aren't they?

Right: since they are not tracked by git, they were not removed. And since the src/sage/libs/cypari2/.gitignore file was removed, they are no longer ignored.

comment:72 in reply to: ↑ 69 Changed 2 years ago by jdemeyer

Replying to jhpalmieri:

Should it be the job of the CyPari2 spkg-install file to delete those?

Certainly not (separation of concerns: the cypari2 package and the Sage library are two independent things).

But src/setup.py could delete those files.

Note: See TracTickets for help on using tickets.