Opened 17 months ago
Closed 8 months ago
#28918 closed enhancement (invalid)
Make giacpy_sage a standard package
Reported by:  slelievre  Owned by:  

Priority:  major  Milestone:  sageduplicate/invalid/wontfix 
Component:  packages: optional  Keywords:  
Cc:  frederichan, ghmwageringel, slelievre, vdelecroix  Merged in:  
Authors:  Reviewers:  Markus Wageringel  
Report Upstream:  N/A  Work issues:  
Branch:  public/giacpy_sage0.7 (Commits, GitHub, GitLab)  Commit:  7a4bdb93fbf73c586b4e0cb8ee5235334b284d8b 
Dependencies:  Stopgaps: 
Description (last modified by )
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
Home:
Upstream upgrade needed (2 doctests failure with python3):
Note: this ticket was made obsolete by
 #29171 "Move giacpy_sage into sage source code" (merged in Sage 9.2.beta10)
Change History (22)
comment:1 Changed 17 months ago by
 Description modified (diff)
 Milestone changed from sage9.0 to sage9.1
comment:2 followup: ↓ 3 Changed 15 months ago by
comment:3 in reply to: ↑ 2 Changed 15 months ago by
Replying to ghmwageringel:
What is required to make this happen?
Usually a vote on sagedevel. 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.
comment:4 Changed 15 months ago by
Launched a poll on sagedevel:
comment:5 Changed 15 months ago by
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.
comment:6 Changed 15 months ago by
 Branch set to public/giacpy_sage0.7
 Commit set to 85e78011637ab7fc553e3d1dd726311676412ec4
 Description modified (diff)
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_sage0.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.
comment:7 Changed 15 months ago by
Something for upstream, I fail to compile giacpy_sage0.7.0 with the latest giac (1.5.0.85)
building 'giacpy_sage' extension creating /dev/shm/portage/devpython/giacpy_sage0.7.0/work/giacpy_sage0.7.0python3_7/temp.linuxx86_643.7 x86_64pclinuxgnug++ march=native O2 pipe fPIC I/usr/lib/python3.7/sitepackages/cysignals I/dev/shm/portage/devpython/giacpy_sage0.7.0/work/giacpy_sage0.7.0 I/usr/lib/python3.7/sitepackages/sage/libs/ntl I/usr/lib/python3.7/sitepackages/sage/cpython I/usr/include I/usr/lib/python3.7/sitepackages I/usr/lib/python3.7/sitepackages/sage/ext I/usr/include/python3.7m I/usr/lib/python3.7/sitepackages/numpy/core/include I/usr/include/python3.7m c giacpy_sage.cpp o /dev/shm/portage/devpython/giacpy_sage0.7.0/work/giacpy_sage0.7.0python3_7/temp.linuxx86_643.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/sitepackages/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/devpython/giacpy_sage0.7.0/work/giacpy_sage0.7.0/giacpy_sage.pxd tree = Parsing.p_module(s, pxd, full_module_name) error: command 'x86_64pclinuxgnug++' failed with exit status 1
Any ideas?
comment:8 Changed 15 months ago by
I actually have the same issue with giacpy_sage 0.6.8.
comment:9 Changed 15 months ago by
Thank you François, it's a missing include. I will fix it in giacpy. It seems that between giac1.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>
comment:10 Changed 15 months ago by
 Commit changed from 85e78011637ab7fc553e3d1dd726311676412ec4 to 7a4bdb93fbf73c586b4e0cb8ee5235334b284d8b
Branch pushed to git repo; I updated commit sha1. New commits:
7a4bdb9  update to giacpy_sage0.7.1 to fix a missing include with giac >1.5.0.75

comment:11 followup: ↓ 12 Changed 15 months ago by
 Description modified (diff)
the missing header with giac >1.5.0.75 reported by francois, should be fixed in giacpy_sage0.7.1 I have update the branch accordingly.
comment:12 in reply to: ↑ 11 Changed 15 months ago by
Replying to frederichan:
the missing header with giac >1.5.0.75 reported by francois, should be fixed in giacpy_sage0.7.1 I have update the branch accordingly.
Works now. Thanks!
comment:13 Changed 13 months ago by
 Milestone changed from sage9.1 to sage9.2
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.
comment:14 Changed 10 months ago by
See also #29552
comment:15 Changed 9 months ago by
 Cc vdelecroix added
 Priority changed from major to critical
comment:16 Changed 9 months ago by
 Status changed from new to needs_info
comment:17 followup: ↓ 19 Changed 9 months ago by
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:
 build sagelib
 build giacpy_sage
 optional: test giacpy_sage
 build sagelib documentation
 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.
comment:18 Changed 9 months ago by
 Milestone changed from sage9.2 to sageduplicate/invalid/wontfix
 Status changed from needs_info to needs_review
Yes, I agree that #29171, merging into sagelib, is the way to go.
comment:19 in reply to: ↑ 17 Changed 9 months ago by
comment:20 Changed 9 months ago by
 Reviewers set to Markus Wageringel
 Status changed from needs_review to positive_review
comment:21 Changed 8 months ago by
 Priority changed from critical to major
comment:22 Changed 8 months ago by
 Description modified (diff)
 Resolution set to invalid
 Status changed from positive_review to closed
What is required to make this happen?