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

Cython version requirements (are varying for different Python releases) #1242

Open tillea opened 10 months ago

tillea commented 10 months ago

Hi, Debian intends to migrate to Python 3.12. In a bug report some incompatibility with this Python version was stated. It would be great if you could make pysam compatible with the latest Python version. Kind regards, Andreas.

jmarshall commented 10 months ago

The linked build log indicates that a version of Cython that is not compatible with Python 3.12 has been used.

jmarshall commented 10 months ago

Pysam's Cython requirement was bumped to >=0.29.12 when Python 3.8 was new, because that version was the first to adjust to some CPython 3.8 API changes:

0.29.12 (2019-07-07) * Fix compile error in CPython 3.8b2 regarding the PyCode_New() signature.

For Python 3.10 onwards, in fact you need at least 0.29.24:

0.29.24 (2021-07-14) * The tracing code was adapted to work with CPython 3.10.

And for Python 3.12, you need at least 3.0:

3.0.0 (2023-07-17) * cython/cython#5450, not explicitly mentioned in CHANGES.rst (and tracing still doesn't actually work in 3.12)

(The linked full build log shows that Debian have tried to use Cython 0.29.36-1ubuntu2 with Python 3.12.)

So it's probably time we bumped our Cython requirements, or listed them per-version as something like (modulo syntax):

Cython>=0.29.12,<4
Cython>=0.29.24 ; python_version >= "3.10"
Cython>=3.0.0 ; python_version >= "3.12"

Or this can be simplified as we drop older Python versions in a similar time frame.

Many of these excess requirements are due to tracing, because pysam uses # cython: profile=True in its .pyx files even though probably vanishingly few of our users are actually using tracing. I have been meaning to remove that for a while, or at least make it configurable (and default to off) in setup() instead of hard-coding it via Cython directives. Without tracing, our Cython version requirements could also be greatly simplified.

Nonetheless, IMHO it's rapidly become foolish to use anything earlier than Cython 3 anyway.

tillea commented 10 months ago

Debian bug was closed with fixed with cython 3.0.5 Thanks a lot for your patience, Andreas.