search

pysam-developers / pysam

Pysam is a Python package for reading, manipulating, and writing genomics data such as SAM/BAM/CRAM and VCF/BCF files. It's a lightweight wrapper of the HTSlib API, the same one that powers samtools, bcftools, and tabix.
https://pysam.readthedocs.io/en/latest/
MIT License
774 stars 274 forks source link

Remove `requires` from `setup()` etc (combines 1216 & 1218) #1219

Closed jmarshall closed 1 year ago

jmarshall commented 1 year ago

This PR combines #1216 and #1218 and some other minor changes.

  1. As noted on PR #1216, this requires entry in setup.py's setup(…) call is listed as deprecated in the setuptools documentation, which recommends using pyproject.toml instead — which we already do. And it seems that this requires entry actually doesn't do anything any more anyway! It adds a Requires: cython (>=0.29.12) line to the resulting wheel's pysam-x.y.z.dist-info/METADATA file, but that's a v1.1 item that was superseded in 2005 and I suspect nothing actually acts on it now.

    So it would be best to remove this line. If we do that, it would be best to update the setuptools requirement to v61.0.0, which is documented as the version that introduced robust pyproject.toml support.

  2. Add Cython <4 version constraint, as discussed on #1217.

  3. Remove .python-version, which appears to have been inadvertently committed when tox.ini was added. Pysam works with more Python versions than listed in that file, so there's not really anything to be gained by having it.

  4. Consolidate tox settings in pyproject.toml instead of its own config file, which requires tox 4.0+.

The only part of this that worries me a little is the setuptools bump to v61.0.0, which is only 18 months old. However most users will be installing via conda or wheels, so setuptools is immaterial for them. Our existing 59.0 requirement is already beyond the system version on all major Linux distro releases except Ubuntu 22.04, so most developers already need to update their system setuptools installation.

(Re wheels: there are a few gaps in 0.21.0's collection of wheels, in particular there are no ARM macOS wheels. I have a followup PR that revamps the cibuildscript scripts to fill these gaps.)

jmarshall commented 1 year ago

In the end, the fact that our CI uses python setup.py build without constraints and hence may receive an already installed setuptools that is an earlier version than v61.0 or even v59.0 (for example, py38 is currently given v56.0.0) and still builds successfully means that this doesn't matter too much.

So we can raise the constraint and pip etc builds will generally install the latest setuptools regardless of whether the bound is v59 or v61. Hence this bump should be fine.