-
-
Notifications
You must be signed in to change notification settings - Fork 25.8k
Do not pre-cythonize when calling sdist #15567
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
Conversation
This means that cython must be installed manually first when building scikit-t learn from a source tarball for future releases. In case Cython is missing or too old, an informative error message is raised.
So just to be sure, this PR will only affect people who install sklearn by downloading the source distribution right ? |
Yes. For users building from a git clone, it will also give a more informative error message when Cython is missing or too old. |
I also fixed the ability to do |
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.
Awesome, thanks a bunch @ogrisel
thanks @ogrisel ! |
Just realized: should we add a what's new entry to mention that now cython is required when building from source distribution ? |
Not sure about a whats_new entry. I'm happy with or without. |
We are basing our minimum Cython version on the newest version on conda supporting Python 3.5. Can we try to keep the Cython version up to date with the latest Cython release? |
Sounds like a good idea to me. We're only doing that to have a passing CI, as long as the CI passes, I'm happy to have the latest possible Cython. |
Pre-generated c/cpp source files from Cython are not guaranteed to be forward with future Python releases. This PR changes the setup.py to always require a recent version of Cython when building scikit-learn from source.
This is a variant of #15533 that does not try to attempt to automatically install the build dependencies (numpy, scipy and cython) using a pyproject.toml file because of how pip current work.
See the discussion in #15533 for more details.