-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[WIP] Use PEP 517 Build Isolation #4312
Conversation
@@ -0,0 +1,3 @@ | |||
[build-system] | |||
requires = ["setuptools", "wheel", "numpy==1.14.5", "cython==0.29.0"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have pinned numpy 1.14.5 as it's the first numpy version with py37 wheels on pypi. I have no idea what a good Cython pin is, there is no reason why we couldn't go very new here if we needed to.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This works for python 3.8?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the strategy is generally to choose the minimum NumPy that is both compatible with scikit-image and for which wheels exist. That is why SciPy has all of the following cases:
https://github.com/scipy/scipy/blob/6644a54ff862e4d90d7f9bc43ece9b0b1f681861/pyproject.toml#L5-L13
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, I think Cython >= 0.29.13 is the minimum compatible with Python 3.8.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes:
$ python --version
Python 3.8.0
$ pip freeze
pkg-resources==0.0.0
$ pip install .
worked fine.
I assume it's compiled numpy and cython from source.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know that this build isolation used to rely on wheels existing, but it seems from this experiment that that is no longer the case. I wonder if I can find a pip reference for that somewhere.
sys.exit(1) | ||
from numpy.distutils.core import setup | ||
extra = {'configuration': configuration} | ||
# Do not try and upgrade larger dependencies |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feels very weird to me, but I have left it alone.
Description
This is following on from a discussion in #4261
This works for me currently when installing from a checkout in a completely clean venv. What I am unclear on at this point is how the C files are generated for the sdist, I am proposing that we remove that behaviour and have people generate the C files during build (with a pinned known version of Cython).
For reviewers
later.
__init__.py
.doc/release/release_dev.rst
.@meeseeksdev backport to v0.14.x