Make giacpy_sage a standard package

[slel]

This is to make giacpy_sage a standard SPKG.

It provides Cython bindings to Giac and has been
an optional SPKG for several years, initially as giacpy,
then since July 2016 as giacpy_sage.

Making it standard would be a step toward replacing
all pexpect calls to Giac by giacpy_sage calls,
as suggested in

• ​#28913 comment:3

Home:

• https://gitlab.math.univ-paris-diderot.fr/han/giacpy-sage/

Upstream upgrade needed (2 doctests failure with python3):

• http://webusers.imj-prg.fr/~frederic.han/xcas/giacpy/sage/giacpy_sage-0.7.1.tar.gz

Note: this ticket was made obsolete by

• Move giacpy_sage into sage source code #29171 "Move giacpy_sage into sage source code" (merged in Sage 9.2.beta10)

CC: @frederichan-IMJPRG @mwageringel @slel @videlec

Component: packages: optional

Branch/Commit: public/giacpy_sage-0.7 @ 7a4bdb9

Reviewer: Markus Wageringel

Issue created by migration from https://trac.sagemath.org/ticket/28918

[mwageringel]

comment:2

What is required to make this happen?

[kiwifb]

comment:3

Replying to @mwageringel:

What is required to make this happen?

Usually a vote on sage-devel. The package has been optional long enough that it should be the only requirement.

For info, the only case new packages go straight to standard without vote or questions is when they are new dependency of a standard package.

[slel]

comment:4

Launched a poll on sage-devel:

• https://groups.google.com/d/topic/sage-devel/uYXGzG_py28/discussion

[frederichan-IMJPRG]

comment:5

Thank you for this ticket.
NB: if the vote is favorable we need to remove all the:

optional - giacpy_sage

in the comments of the doctests in the following files:

src/sage/libs/giac.py
src/sage/rings/polynomial/multi_polynomial_ideal.py

NB: the random and long time comments should stay.

There is also a flag in the following

build/pkgs/giacpy_sage/type

but I don't know if the ticket should change it or let the Release Manager do it himself.

[frederichan-IMJPRG]

Commit: 85e7801

[frederichan-IMJPRG]

comment:6

I have found 2 doctest failure in sage 9.1.beta3 due to python3. So we also need to upgrade the upstream source to giacpy_sage-0.7.0.

I have started a new public branch with this upstream fix. But let all the optional flags in the sage code. If the vote turn out to be positive we will have to remove them.

[frederichan-IMJPRG]

Author: Frederic Han

[frederichan-IMJPRG]

Branch: public/giacpy_sage-0.7

[kiwifb]

comment:7

Something for upstream, I fail to compile giacpy_sage-0.7.0 with the latest giac (1.5.0.85)

building 'giacpy_sage' extension
creating /dev/shm/portage/dev-python/giacpy_sage-0.7.0/work/giacpy_sage-0.7.0-python3_7/temp.linux-x86_64-3.7
x86_64-pc-linux-gnu-g++ -march=native -O2 -pipe -fPIC -I/usr/lib/python3.7/site-packages/cysignals -I/dev/shm/portage/dev-python/giacpy_sage-0.7.0/work/giacpy_sage-0.7.0 -I/usr/lib/python3.7/site-packages/sage/libs/ntl -I/usr/lib/python3.7/site-packages/sage/cpython -I/usr/include -I/usr/lib/python3.7/site-packages -I/usr/lib/python3.7/site-packages/sage/ext -I/usr/include/python3.7m -I/usr/lib/python3.7/site-packages/numpy/core/include -I/usr/include/python3.7m -c giacpy_sage.cpp -o /dev/shm/portage/dev-python/giacpy_sage-0.7.0/work/giacpy_sage-0.7.0-python3_7/temp.linux-x86_64-3.7/giacpy_sage.o -std=c++11
In file included from giacpy_sage.cpp:665:
misc.h: In function ‘void archivegen(std::string, const giac::gen&, const giac::context*)’:
misc.h:104:15: error: variable ‘std::ofstream of’ has initializer but incomplete type
  104 |   ofstream of(filename.c_str());
      |               ^~~~~~~~
misc.h: In function ‘giac::gen unarchivegen(std::string, const giac::context*)’:
misc.h:110:14: error: variable ‘std::ifstream f’ has initializer but incomplete type
  110 |   ifstream f(filename.c_str());
      |              ^~~~~~~~
/usr/lib/python3.7/site-packages/Cython/Compiler/Main.py:369: FutureWarning: Cython directive 'language_level' not set, using 2 for now (Py2). This will change in a later release! File: /dev/shm/portage/dev-python/giacpy_sage-0.7.0/work/giacpy_sage-0.7.0/giacpy_sage.pxd
  tree = Parsing.p_module(s, pxd, full_module_name)
error: command 'x86_64-pc-linux-gnu-g++' failed with exit status 1

Any ideas?

[kiwifb]

comment:8

I actually have the same issue with giacpy_sage 0.6.8.

[frederichan-IMJPRG]

comment:9

Thank you François, it's a missing include. I will fix it in giacpy.
It seems that between giac-1.5.0.63 and 1.5.0.75 fstream was removed in several places:

fred@localhost src]$ grep "<fstream>" *.h
global.h://#include <fstream>
Graph3d.h:#include <fstream>
graphe.h:#include <fstream>
Graph.h:#include <fstream>
help.h://#include <fstream>
monomial.h://#include <fstream>
opengl.h:#include <fstream>
poly.h://#include <fstream>

[fred@localhost src]$ grep "<fstream>" ~/dev/sage/develop/sage/local/include/giac/*.h
/home/fred/dev/sage/develop/sage/local/include/giac/global.h:#include <fstream>
/home/fred/dev/sage/develop/sage/local/include/giac/graphe.h:#include <fstream>
/home/fred/dev/sage/develop/sage/local/include/giac/help.h:#include <fstream>
/home/fred/dev/sage/develop/sage/local/include/giac/monomial.h:#include <fstream>
/home/fred/dev/sage/develop/sage/local/include/giac/poly.h:#include <fstream>

[sagetrac-git]

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

7a4bdb9

update to giacpy_sage-0.7.1 to fix a missing include with giac >1.5.0.75

[sagetrac-git]

Changed commit from 85e7801 to 7a4bdb9

[frederichan-IMJPRG]

comment:11

the missing header with giac >1.5.0.75 reported by francois, should be fixed in giacpy_sage-0.7.1
I have update the branch accordingly.

[kiwifb]

comment:12

Replying to @frederichan-IMJPRG:

the missing header with giac >1.5.0.75 reported by francois, should be fixed in giacpy_sage-0.7.1
I have update the branch accordingly.

Works now. Thanks!

[mkoeppe]

comment:13

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.

[mkoeppe]

comment:14

See also #29552

[kiwifb]

comment:17

I have issues with this going to standard by just flipping the switch. It risks to become a circular dependency madness.

Currently giacpy_sage requires sage at build time to figure out the include directory by importing sage_include_directory from sage.env. That part is not needed and should probably be simplified. If you are installing via sage -i the environment should be set correctly for finding the headers. If you are on a distro they also are in reach. Similarly library_dirs doesn't need to be set (in fact the current setup is annoying on systems with lib/lib64 split). But sage is needed when compiling the .pyx files as it imports a fair deal of sage bits.

sage is needed at runtime and test time.

If sage needs the package at runtime or to build the documentation you create a scenario where building sage look like:

1. build sagelib
2. build giacpy_sage
3. optional: test giacpy_sage
4. build sagelib documentation
5. test sagelib

So giacpy_sage is inserting itself in the middle of the traditional build cycle.

sage_brial look superficially similar but in its case it can be installed independently of sage so it can go as step 1 (it cannot be tested however). sage_brial should in fact be merged into sagelib and I believe merging giacpy_sage inside sagelib is the only way to get out what looks like circular dependency in the future.

[mkoeppe]

comment:18

Yes, I agree that #29171, merging into sagelib, is the way to go.

[mkoeppe]

comment:19

Replying to @kiwifb:

sage_brial look superficially similar but in its case it can be installed independently of sage so it can go as step 1 (it cannot be tested however). sage_brial should in fact be merged into sagelib

This is now #30332

[mwageringel]

Changed author from Frederic Han to none

[mwageringel]

Reviewer: Markus Wageringel