Do not pre-cythonize when calling sdist#15567
Do not pre-cythonize when calling sdist#15567jeremiedbb merged 3 commits intoscikit-learn:masterfrom
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 |
adrinjalali
left a comment
There was a problem hiding this comment.
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.