Opened 9 months ago

Closed 8 months ago

#31240 closed enhancement (fixed)

sage-flatsurf 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: fba93c5 (Commits, GitHub, GitLab) Commit: fba93c552ce7f7db2afade8ca427f8b67977325e
Dependencies: #31224 Stopgaps:

Status badges

Change History (18)

comment:1 Changed 9 months ago by vdelecroix

  • Branch set to u/vdelecroix/31240
  • Cc mkoeppe added
  • Commit set to cfb0365a77a88f45ffa09edc087532072cb3bafe

comment:2 Changed 9 months ago by mkoeppe

type is missing, and probably you should add surface_dynamics from #31224 also in dependencies to avoid race conditions on installation

comment:3 follow-up: Changed 9 months ago by vdelecroix

surface_dynamics is a runtime dependency. No need at install.

comment:4 Changed 9 months ago by vdelecroix

  • Dependencies set to #31224

comment:5 Changed 9 months ago by git

  • Commit changed from cfb0365a77a88f45ffa09edc087532072cb3bafe to 9fc8626f46314c5cfd4c15ec10fa365646c48b58

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

026d5af31240: type file
9fc862631240: add surface_dynamics as a depenency

comment:6 Changed 9 months ago by vdelecroix

  • Status changed from new to needs_review

comment:7 in reply to: ↑ 3 Changed 9 months ago by mkoeppe

Replying to vdelecroix:

surface_dynamics is a runtime dependency. No need at install.

Note that by listing it in requirements.txt, it will be installed

comment:8 Changed 9 months ago by vdelecroix

Note that it is both in dependencies and requirements.txt. Is it how it should be?

comment:9 Changed 9 months ago by mkoeppe

The one in requirements.txt is redundant because the Sage package dependencies already make sure that surface_dynamics is installed first.

comment:10 Changed 9 months ago by vdelecroix

Ok let me remove it from there then.

comment:11 Changed 9 months ago by git

  • Commit changed from 9fc8626f46314c5cfd4c15ec10fa365646c48b58 to c4531cfdb0650d2f979c766e71eaebaebe64499e

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

c4531cf31240: remove surface_dynamics dependency from requirements.txt

comment:12 Changed 9 months ago by mkoeppe

Could you merge the branch of #31224 please to facilitate testing

comment:13 Changed 9 months ago by git

  • Commit changed from c4531cfdb0650d2f979c766e71eaebaebe64499e to 4676aff98ba2b04ef34bbfdb430982abab230ff5

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

1dad15331224: add surface-dynamics as an optional pip package
4676aff31240: merge ticket #31224 branch

comment:14 Changed 9 months ago by mkoeppe

Could you put the title in SPKG.rst to the new format that includes a 1-line description?

Also it would probably be helpful to expand the description a tiny bit

comment:15 Changed 9 months ago by git

  • Commit changed from 4676aff98ba2b04ef34bbfdb430982abab230ff5 to fba93c552ce7f7db2afade8ca427f8b67977325e

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

fba93c531240: more in SPKG.rst

comment:16 Changed 9 months ago by mkoeppe

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

comment:17 Changed 9 months ago by vdelecroix

Thanks.

comment:18 Changed 8 months ago by vbraun

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