Remove requires from setup() etc (combines 1216 & 1218)
#1219
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Collect settings in pyproject.toml instead of bespoke files. (Eventually we may render setup.cfg unnecessary too.) Fix list syntax and set REF_PATH environment variable to speed up running pytest by avoiding network CRAM reference requests. Remove .python-version, which was probably inadvertently added. Ignore it and tidy up ignoring Sphinx output.
…opers#1216) Unlike install_requires and setup_requires, it seems that this no longer really does anything. 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. As pyproject.toml specifies the requirement anyway, we can just delete this. Now that we depend on settings in pyproject.toml, update the setuptools requirement to v61.0.0, which is documented as the version that introduced robust pyproject.toml support. The distutils.errors fallback was already unnecessary with a requirement for setuptools >= v59.0.0, so remove it. Co-Authored-By: John Marshall <[email protected]>
…elopers#1218) If there is ever a Cython 4 release, the major version number bump will be due to some incompatible change. While we fully plan to support such a Cython version in the contemporaneous future pysam, today's releases may not be compatible with it. Mark them as such -- to be revisited during such a Cython's development cycle and pre-releases. Fixes pysam-developers#1217.
While pyproject.toml specifies a version of setuptools that does provide LinkError et al, our CI uses `python setup.py build` without constraints so can use an already installed setuptools that may be an earlier version. For example, py38 tries to use v56.0.0. For now, reinstate the fallback to work around this.
This was referenced Sep 10, 2023
Closed
|
In the end, the fact that our CI uses 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. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR combines #1216 and #1218 and some other minor changes.
As noted on PR Move Cython to build dependencies in
setup.py#1216, thisrequiresentry in setup.py'ssetup(…)call is listed as deprecated in the setuptools documentation, which recommends using pyproject.toml instead — which we already do. And it seems that thisrequiresentry actually doesn't do anything any more anyway! It adds aRequires: 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.
Add
Cython <4version constraint, as discussed on more stringent cython version pin #1217.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.
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.)