Opened 5 months ago
Closed 5 months ago
#31224 closed enhancement (fixed)
Add surfacedynamics as a pip package
Reported by:  vdelecroix  Owned by:  

Priority:  major  Milestone:  sage9.3 
Component:  packages: optional  Keywords:  
Cc:  mkoeppe  Merged in:  
Authors:  Vincent Delecroix  Reviewers:  Matthias Koeppe 
Report Upstream:  N/A  Work issues:  
Branch:  3a807c0 (Commits, GitHub, GitLab)  Commit:  3a807c00d47da76de65cd657aa8c662accea9a4f 
Dependencies:  Stopgaps: 
Description (last modified by )
See #31164. Upstream code at https://gitlab.com/videlec/surface_dynamics and https://pypi.org/project/surfacedynamics/.
Change History (20)
comment:1 Changed 5 months ago by
 Description modified (diff)
comment:2 Changed 5 months ago by
 Branch set to u/vdelecroix/31224
 Cc mkoeppe added
 Commit set to 35e214cffb746ad06eeb0a24455668ad3d0a5c46
 Status changed from new to needs_review
comment:3 Changed 5 months ago by
Please change SPKG.txt to SPKG.rst; also the title should start with surface_dynamics
(underscore, not dash  this is the SPKG name)
And a requirements.txt
file needs to be added  see https://doc.sagemath.org/html/en/developer/packaging.html#packagesourcetypes
comment:4 Changed 5 months ago by
 Commit changed from 35e214cffb746ad06eeb0a24455668ad3d0a5c46 to 1dad153248df6d4c9dfa16e683d1eb9ef3e2ee2e
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
1dad153  31224: add surfacedynamics as an optional pip package

comment:5 Changed 5 months ago by
Thanks. Done.
comment:6 Changed 5 months ago by
 Reviewers set to Matthias Koeppe
 Status changed from needs_review to positive_review
comment:7 Changed 5 months ago by
 Status changed from positive_review to needs_work
This (and probably other packages just added) will need additional dependencies  see https://github.com/mkoeppe/sage/runs/1712141788?check_suite_focus=true
[surface_dynamics] installing. Log file: /sage/logs/pkgs/surface_dynamics.log [surface_dynamics] error installing, exit status 1. End of log file: [surface_dynamics] Collecting surface_dynamics [surface_dynamics] Downloading surface_dynamics0.4.5.tar.gz (11.3 MB) [surface_dynamics] ERROR: Command errored out with exit status 1: [surface_dynamics] command: /sage/local/bin/python3 c 'import sys, setuptools, tokenize; sys.argv[0] = '"'"'/tmp/pipinstall09x74vlr/surfacedynamics_62aa1d77ac5249259cfc2622814d3adf/setup.py'"'"'; __file__='"'"'/tmp/pipinstall09x74vlr/surfacedynamics_62aa1d77ac5249259cfc2622814d3adf/setup.py'"'"';f=getattr(tokenize, '"'"'open'"'"', open)(__file__);code=f.read().replace('"'"'\r\n'"'"', '"'"'\n'"'"');f.close();exec(compile(code, __file__, '"'"'exec'"'"'))' egg_info eggbase /tmp/pippipegginfo0g71blv2 [surface_dynamics] cwd: /tmp/pipinstall09x74vlr/surfacedynamics_62aa1d77ac5249259cfc2622814d3adf/ [surface_dynamics] Complete output (15 lines): [surface_dynamics] Traceback (most recent call last): [surface_dynamics] File "/tmp/pipinstall09x74vlr/surfacedynamics_62aa1d77ac5249259cfc2622814d3adf/setup.py", line 8, in <module> [surface_dynamics] import sage.all [surface_dynamics] ModuleNotFoundError: No module named 'sage' [surface_dynamics] [surface_dynamics] During handling of the above exception, another exception occurred: [surface_dynamics] [surface_dynamics] Traceback (most recent call last): [surface_dynamics] File "<string>", line 1, in <module> [surface_dynamics] File "/tmp/pipinstall09x74vlr/surfacedynamics_62aa1d77ac5249259cfc2622814d3adf/setup.py", line 10, in <module> [surface_dynamics] raise ValueError("this package currently installs only inside SageMath (http://www.sagemath.org)\n" [surface_dynamics] ValueError: this package currently installs only inside SageMath (http://www.sagemath.org) [surface_dynamics] If you are using Ubuntu with Sage installed from the official apt repository, run [surface_dynamics] first in a console "$ source /usr/share/sagemath/bin/sageenv" [surface_dynamics] [surface_dynamics]  [surface_dynamics] ERROR: Command errored out with exit status 1: python setup.py egg_info Check the logs for full command output.
comment:8 Changed 5 months ago by
How do I set sage Python library as a dependency!?
comment:9 Changed 5 months ago by
For this very specific package, it is needed at install time (Cython compilation). For pure Python it will only be a runtime dependency.
comment:10 Changed 5 months ago by
I think an orderonly dependency on $(SAGE_RUNTIME)
will do the trick.
See build/pkgs/sage_numerical_backends_coin/dependencies
for an example.
comment:11 Changed 5 months ago by
It is written $(SAGERUNTIME)
in this file, not $(SAGE_RUNTIME)
... Where is it misspelled?
comment:12 Changed 5 months ago by
SAGERUNTIME
is correct. Sorry for the confusion
comment:13 Changed 5 months ago by
 Commit changed from 1dad153248df6d4c9dfa16e683d1eb9ef3e2ee2e to 7e0a7df145b56090ec17b3bdd62d93540f961a0f
Branch pushed to git repo; I updated commit sha1. New commits:
7e0a7df  31224: SAGERUNTIME dependency

comment:14 Changed 5 months ago by
 Status changed from needs_work to needs_review
comment:15 Changed 5 months ago by
Wouldn't it be safer with a normal dependency? Namely I have Cython inheritance there. If a Cython object such as sage.structure.element.Element
is changed in the Sage library, it might end up with wrong compiled Cython code.
comment:16 Changed 5 months ago by
The problem with a normal dependency is that sagelib
is always built (dependency on FORCE
in build/pkgs/sagelib/dependencies
), which would trigger an unconditional rebuild of this package. That's why I have included a number of key sage source files in build/pkgs/sage_numerical_backends_coin/dependencies
. (See #29711 for a related discussion)
comment:17 Changed 5 months ago by
Great: $(SAGE_SRC)/sage/cpython/string.pxd
is allowed!
comment:18 Changed 5 months ago by
 Commit changed from 7e0a7df145b56090ec17b3bdd62d93540f961a0f to 3a807c00d47da76de65cd657aa8c662accea9a4f
Branch pushed to git repo; I updated commit sha1. New commits:
3a807c0  31224: dependency on sage sources

comment:19 Changed 5 months ago by
 Status changed from needs_review to positive_review
Looking great.
comment:20 Changed 5 months ago by
 Branch changed from u/vdelecroix/31224 to 3a807c00d47da76de65cd657aa8c662accea9a4f
 Resolution set to fixed
 Status changed from positive_review to closed
New commits:
31224: add surfacedynamics as an optional pip package