Opened 5 months ago

Closed 5 months ago

#31224 closed enhancement (fixed)

Add surface-dynamics as a pip package

Reported by: vdelecroix Owned by:
Priority: major Milestone: sage-9.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:

Status badges

Change History (20)

comment:1 Changed 5 months ago by vdelecroix

  • Description modified (diff)

comment:2 Changed 5 months ago by vdelecroix

  • Branch set to u/vdelecroix/31224
  • Cc mkoeppe added
  • Commit set to 35e214cffb746ad06eeb0a24455668ad3d0a5c46
  • Status changed from new to needs_review

New commits:

35e214c31224: add surface-dynamics as an optional pip package

comment:3 Changed 5 months ago by mkoeppe

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#package-source-types

comment:4 Changed 5 months ago by git

  • Commit changed from 35e214cffb746ad06eeb0a24455668ad3d0a5c46 to 1dad153248df6d4c9dfa16e683d1eb9ef3e2ee2e

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

1dad15331224: add surface-dynamics as an optional pip package

comment:5 Changed 5 months ago by vdelecroix

Thanks. Done.

comment:6 Changed 5 months ago by mkoeppe

  • Reviewers set to Matthias Koeppe
  • Status changed from needs_review to positive_review

comment:7 Changed 5 months ago by mkoeppe

  • 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_dynamics-0.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/pip-install-09x74vlr/surface-dynamics_62aa1d77ac5249259cfc2622814d3adf/setup.py'"'"'; __file__='"'"'/tmp/pip-install-09x74vlr/surface-dynamics_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 --egg-base /tmp/pip-pip-egg-info-0g71blv2
  [surface_dynamics]            cwd: /tmp/pip-install-09x74vlr/surface-dynamics_62aa1d77ac5249259cfc2622814d3adf/
  [surface_dynamics]       Complete output (15 lines):
  [surface_dynamics]       Traceback (most recent call last):
  [surface_dynamics]         File "/tmp/pip-install-09x74vlr/surface-dynamics_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/pip-install-09x74vlr/surface-dynamics_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/sage-env"
  [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 vdelecroix

How do I set sage Python library as a dependency!?

comment:9 Changed 5 months ago by vdelecroix

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 mkoeppe

I think an order-only 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 vdelecroix

It is written $(SAGERUNTIME) in this file, not $(SAGE_RUNTIME)... Where is it misspelled?

comment:12 Changed 5 months ago by mkoeppe

SAGERUNTIME is correct. Sorry for the confusion

comment:13 Changed 5 months ago by git

  • Commit changed from 1dad153248df6d4c9dfa16e683d1eb9ef3e2ee2e to 7e0a7df145b56090ec17b3bdd62d93540f961a0f

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

7e0a7df31224: SAGERUNTIME dependency

comment:14 Changed 5 months ago by vdelecroix

  • Status changed from needs_work to needs_review

comment:15 Changed 5 months ago by vdelecroix

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 mkoeppe

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)

Last edited 5 months ago by mkoeppe (previous) (diff)

comment:17 Changed 5 months ago by vdelecroix

Great: $(SAGE_SRC)/sage/cpython/string.pxd is allowed!

comment:18 Changed 5 months ago by git

  • Commit changed from 7e0a7df145b56090ec17b3bdd62d93540f961a0f to 3a807c00d47da76de65cd657aa8c662accea9a4f

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

3a807c031224: dependency on sage sources

comment:19 Changed 5 months ago by mkoeppe

  • Status changed from needs_review to positive_review

Looking great.

comment:20 Changed 5 months ago by vbraun

  • Branch changed from u/vdelecroix/31224 to 3a807c00d47da76de65cd657aa8c662accea9a4f
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.