#30719 closed enhancement (fixed)

Add build/pkgs/SPKG/install-requires.txt for all Python packages, remove some unneeded packages

Reported by: mkoeppe Owned by:
Priority: critical Milestone: sage-9.3
Component: build Keywords:
Cc: gh-tobiasdiez, isuruf, fbissey, arojas, dimpase Merged in:
Authors: Matthias Koeppe Reviewers: Dima Pasechnik
Report Upstream: N/A Work issues:
Branch: 6ec00dd (Commits, GitHub, GitLab) Commit: 6ec00dd002b41c4fe9b91233f2cebe37c87144d9
Dependencies: Stopgaps:

Status badges

Description (last modified by mkoeppe)

These files:

  • serve as a marker that an SPKG is a Python package (for #29013)
  • may include comments that explain why we reject certain versions
  • provide version constraints (lower and upper bounds)
  • as such, are preparation for #29023: Meta-ticket: In a python 3 build, optionally use system Python packages
  • can be used to generate other metadata such as src/Pipfile, src/setup.cfg [install_requires] (#29041) in the bootstrap phase, to avoid duplicating this information in various places

Change History (33)

comment:1 Changed 13 months ago by mkoeppe

  • Branch set to u/mkoeppe/add_build_pkgs_spkg_install_requires_txt_for_all_python_packages

comment:2 Changed 13 months ago by mkoeppe

  • Commit set to a754e8c70b528e2c010e2d62c9375c874802a8c0

Tobias, before I add more of these, do you think this format is useful for generating other files?


New commits:

a754e8cAdd some build/pkgs/*/install-requires.txt

comment:3 Changed 13 months ago by git

  • Commit changed from a754e8c70b528e2c010e2d62c9375c874802a8c0 to 11ce883bcb57024a33c6b22ca0dc8ceb2674e9c8

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

11ce883Add more build/pkgs/*/install-requires.txt

comment:4 follow-up: Changed 13 months ago by gh-tobiasdiez

So the idea would be to concatenate all these install-requires.txt files to obtain the install_requires section of src/setup.cfg? That should work. Except that cython is a more a compile time dependency ([build-system] requires = ["cython"] in pyproject.toml), right?

I have to admit though that I don't see the benefit of having all these text files over having a single src/setup.cfg (or pyproject for that matter). It feels like it's the same information, just in a different location. In fact, it feels a bit like you are reinventing the wheel. pyproject etc also have support for different kinds of dependencies, e.g. optional, compile time (cython), test (pytest) and build dependencies (tox).

Maybe that's a bit too naïve, but I would approach this as follows: sagelib (everything in /src) is a python library and as such only declares minimum dependencies and their requirements, trying not to pin versions (except if it's really necessary). I think, the most modern way would be to do this via a pyproject.toml file. Then sage-the-application (the actual thing used by the enduser) is a python application and thus includes sagelib as a library and needs to satisfy its dependencies (this is the point where you have to specify in some way the exact version you want to use). sage-the library should be completely independent of sage-the-application (i.e. it should work to have them in different git repositories).

Last edited 13 months ago by gh-tobiasdiez (previous) (diff)

comment:5 in reply to: ↑ 4 ; follow-up: Changed 13 months ago by mkoeppe

Replying to gh-tobiasdiez:

So the idea would be to concatenate all these install-requires.txt files to obtain the install_requires section of src/setup.cfg?

Yes.

That should work. Except that cython is a more a compile time dependency ([build-system] requires = ["cython"] in pyproject.toml), right?

Right; but sagelib also exposes cython at runtime for interactive use.

I have to admit though that I don't see the benefit of having all these text files over having a single src/setup.cfg (or pyproject for that matter). It feels like it's the same information, just in a different location.

With modularization, I expect to see dozens of setup.cfg files for the various subset distributions. Basically, I don't want to duplicate version ranges between these.

In fact, it feels a bit like you are reinventing the wheel. pyproject etc also have support for different kinds of dependencies, e.g. optional, compile time (cython), test (pytest) and build dependencies (tox).

What I want to do is define version ranges in one place for all of the Sage project.

Distinguishing the kinds of dependencies (build, test, ...) should be orthogonal -- and the mechanism in #29041 should be able to put these version ranges into whatever file/format is needed.

(I don't think it would add sufficient value to Sage to have different version ranges for a given package in different roles, like "we need Cython >= 0.29.21 for build, but any Cython >= 0.27 is fine for runtime.)

sage-the library should be completely independent of sage-the-application (i.e. it should work to have them in different git repositories).

I don't like to go into this direction because it would add a big hurdle to our developer community. Having one git repository is key.

comment:6 in reply to: ↑ 5 ; follow-ups: Changed 13 months ago by gh-tobiasdiez

Replying to mkoeppe:

I have to admit though that I don't see the benefit of having all these text files over having a single src/setup.cfg (or pyproject for that matter). It feels like it's the same information, just in a different location.

With modularization, I expect to see dozens of setup.cfg files for the various subset distributions. Basically, I don't want to duplicate version ranges between these.

Sounds like a valid point.

Having one git repository is key.

I agree! What I meant is that it might be helpful to approach it like this, i.e. how would you solve this problem if sage-library would be sitting in a different git repo? I think, this will be helpful e.g for distribution as pipy-installable package where it would be helpful if src/ works independently of the make and build scripts.

One more remark: I would try to make the version range specifications as general as possible, i.e. only add restrictions if its known that that this particular version doesn't work with sage (this in particular applies to the upper limit). Otherwise the sage-library (and its subdistributions) would be harder to use (e.g. if one cannot upgrade/downgrade a dependency because other code relies on it). In this context, it may actually turn out to be helpful if you could could control the dependency versions for each submodule sepereatly, e.g. if ModuleA breaks with a newer version but ModuleB doesn't have these problems.

comment:7 in reply to: ↑ 6 Changed 13 months ago by mkoeppe

Replying to gh-tobiasdiez:

What I meant is that it might be helpful to approach it like this, i.e. how would you solve this problem if sage-library would be sitting in a different git repo? I think, this will be helpful e.g for distribution as pipy-installable package where it would be helpful if src/ works independently of the make and build scripts.

Sounds like we are basically in agreement here. One detail, however, is the bootstrapping phase. The way I see it, after running ./bootstrap, we would have standard Python package source trees (which will not depend on make and build) -- right now, src or build/pkgs/sagelib/src/, but many more after modularization (such as build/pkgs/sage-ntl/src/). By using scripts that are executed during bootstrapping, we can avoid duplicating a lot of information and thus reduce the maintenance burden.

comment:8 in reply to: ↑ 6 Changed 13 months ago by mkoeppe

Replying to gh-tobiasdiez:

I would try to make the version range specifications as general as possible, i.e. only add restrictions if its known that that this particular version doesn't work with sage (this in particular applies to the upper limit). Otherwise the sage-library (and its subdistributions) would be harder to use (e.g. if one cannot upgrade/downgrade a dependency because other code relies on it).

Yes, this is tricky to get right and will probably need some detailed discussion for individual packages. But I do think that some upper bounds on the version are always needed because we do not want our package to stop working if a new major version of some package is released on pypi. For example, Cython has an upcoming 3.x version which we have not tested with at all.

In this context, it may actually turn out to be helpful if you could could control the dependency versions for each submodule separately, e.g. if ModuleA breaks with a newer version but ModuleB doesn't have these problems.

Yes, this is conceivable but I would hope that if this ever becomes necessary, it can be implemented by overriding the default rather than defaulting to duplicate maintenance of the version ranges.

comment:9 Changed 13 months ago by git

  • Commit changed from 11ce883bcb57024a33c6b22ca0dc8ceb2674e9c8 to f9ac70ef321f333f2b7d0a7a080c01899cec4f86

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

85b2dcffor a in *; do if [ -d $a -a ! -r $a/requirements.txt -a ! -r $a/install-requires.txt ]; then if grep -q -i sdh_pip $a/spkg-install.in >&/dev/null ; then ( if [ -r $a/package-version.txt ]; then echo "$a >=$(sed s/[.]p[0-9]$// $a/package-version.txt)"; else echo "$a"; fi) > $a/install-requires.txt && git add $a/install-requires.txt; fi; fi; done
f9ac70eAdd install-requires.txt for setuptools, pip

comment:10 Changed 13 months ago by mkoeppe

Autogenerated for now, setting current version as lower bound

comment:11 Changed 12 months ago by mkoeppe

  • Milestone changed from sage-9.2 to sage-9.3

comment:12 Changed 12 months ago by mkoeppe

  • Cc isuruf fbissey arojas dimpase added
  • Status changed from new to needs_review

I'm hoping to gather version information for our Python packages here.

comment:13 Changed 12 months ago by mkoeppe

  • Authors set to Matthias Koeppe

comment:14 Changed 12 months ago by mkoeppe

  • Description modified (diff)

comment:15 Changed 12 months ago by mkoeppe

  • Priority changed from major to critical

comment:16 Changed 12 months ago by git

  • Commit changed from f9ac70ef321f333f2b7d0a7a080c01899cec4f86 to f6a2c56e0f098dedb0aa3bdbebad1a9d09d549f8

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

f6a2c56Merge tag '9.3.beta1' into t/30719/add_build_pkgs_spkg_install_requires_txt_for_all_python_packages

comment:17 Changed 12 months ago by fbissey

That's a lot of packages to parse :(

Some stuff I can notice from a cursory inspection. I use sphinx 3.2.1 so the restriction <3.2 seem useless. Similarly I currently use networkx 2.5. python_openid was a sagenb dependency, I don't think we need it anymore. I certainly do not have it anymore on my system. Same with itsdangerous.

comment:18 follow-up: Changed 12 months ago by gh-tobiasdiez

The pipenv.lock file added in #30371 is a good indication, too.

The <1.0 constraint for cython seems to be not necessary (at least they are very far off from version 1.0).

comment:19 Changed 12 months ago by git

  • Commit changed from f6a2c56e0f098dedb0aa3bdbebad1a9d09d549f8 to ba1d913e2e340c8ec5c533ecb26d2f3d835b4eb9

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

e61929dbuild/pkgs/sphinx/install-requires.txt: Update from gentoo
8fe0e35build/pkgs/python_openid: Unused, remove
ba1d913build/pkgs/itsdangerous: Unused, remove

comment:20 in reply to: ↑ 18 Changed 12 months ago by mkoeppe

Replying to gh-tobiasdiez:

The <1.0 constraint for cython seems to be not necessary (at least they are very far off from version 1.0).

From comment 8:

For example, Cython has an upcoming 3.x version which we have not tested with at all.

comment:21 Changed 12 months ago by git

  • Commit changed from ba1d913e2e340c8ec5c533ecb26d2f3d835b4eb9 to bb6c4ae829c6c6f55d305e64833a5b85d50fc205

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

bb6c4aebuild/pkgs/tox/install-requires.txt: New

comment:22 Changed 12 months ago by git

  • Commit changed from bb6c4ae829c6c6f55d305e64833a5b85d50fc205 to d50750123c5368874f99a05626f33fade165b246

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

d507501build/pkgs/networkx/install-requires.txt: Update from gentoo

comment:23 Changed 12 months ago by mkoeppe

See also: Upgrade ticket #30611

comment:24 Changed 12 months ago by git

  • Commit changed from d50750123c5368874f99a05626f33fade165b246 to d5e9840e860958f2aafe63280ea822900e722658

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

82d0df3build/pkgs/p_group_cohomology/install-requires.txt: Not on PyPI, remove
d5e9840build/pkgs/pathpy/install-requires.txt: Package removed in #30611, remove

comment:25 Changed 12 months ago by git

  • Commit changed from d5e9840e860958f2aafe63280ea822900e722658 to c3a9352acb3f87d8afdb5d9b0d61611cc30fefaa

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

c3a9352build/pkgs/pathpy/install-requires.txt: Package removed in #30611, remove

comment:26 Changed 12 months ago by git

  • Commit changed from c3a9352acb3f87d8afdb5d9b0d61611cc30fefaa to ebd4610519f9794aef64e8f283adcc827addf6f5

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

ebd4610build/pkgs/pynac/install-requires.txt: New

comment:27 Changed 12 months ago by mkoeppe

  • Summary changed from Add build/pkgs/SPKG/install-requires.txt for all Python packages to Add build/pkgs/SPKG/install-requires.txt for all Python packages, remove some unneeded packages

comment:28 Changed 12 months ago by mkoeppe

Good enough as a first approximation?

comment:29 Changed 12 months ago by mkoeppe

Missing: numpy

comment:30 Changed 12 months ago by git

  • Commit changed from ebd4610519f9794aef64e8f283adcc827addf6f5 to 6ec00dd002b41c4fe9b91233f2cebe37c87144d9

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

6ec00ddbuild/pkgs/{numpy,pillow}/install-requires.txt: New

comment:31 Changed 12 months ago by dimpase

  • Reviewers set to Dima Pasechnik
  • Status changed from needs_review to positive_review

OK, good. (all tests of this branch pass on Fedora 30, too)

comment:32 Changed 12 months ago by mkoeppe

Thanks!

comment:33 Changed 11 months ago by vbraun

  • Branch changed from u/mkoeppe/add_build_pkgs_spkg_install_requires_txt_for_all_python_packages to 6ec00dd002b41c4fe9b91233f2cebe37c87144d9
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.