Skip to content

BLD do not cythonize for sdist #15533

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

Closed
wants to merge 24 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,12 @@ jobs:
- NUMPY_VERSION: 1.11.0
- SCIPY_VERSION: 0.17.0
- MATPLOTLIB_VERSION: 1.5.1
# on conda, this is the latest for python 3.5
# The following places need to be in sync with regard to Cython version:
# - pyproject.toml
# - .circleci config file
# - sklearn/_build_utils/__init__.py
# - advanced installation guide
- CYTHON_VERSION: 0.28.5
- SCIKIT_IMAGE_VERSION: 0.12.3
steps:
Expand Down
2 changes: 1 addition & 1 deletion MANIFEST.in
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,4 @@ recursive-include sklearn *.c *.h *.pyx *.pxd *.pxi *.tp
recursive-include sklearn/datasets *.csv *.csv.gz *.rst *.jpg *.txt *.arff.gz *.json.gz
include COPYING
include README.rst

include pyproject.toml
1 change: 0 additions & 1 deletion build_tools/azure/install.cmd
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ IF "%PYTHON_ARCH%"=="64" (
conda create -n %VIRTUALENV% -q -y python=%PYTHON_VERSION% numpy scipy cython matplotlib wheel pillow joblib

call activate %VIRTUALENV%

IF "%PYTEST_VERSION%"=="*" (
pip install pytest
) else (
Expand Down
20 changes: 17 additions & 3 deletions build_tools/azure/install.sh
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ if [[ "$DISTRIB" == "conda" ]]; then

elif [[ "$DISTRIB" == "ubuntu" ]]; then
sudo add-apt-repository --remove ppa:ubuntu-toolchain-r/test
sudo apt-get update
sudo apt-get install python3-scipy python3-matplotlib libatlas3-base libatlas-base-dev libatlas-dev python3-virtualenv
python3 -m virtualenv --system-site-packages --python=python3 $VIRTUALENV
source $VIRTUALENV/bin/activate
Expand All @@ -88,8 +89,12 @@ elif [[ "$DISTRIB" == "conda-pip-latest" ]]; then
# Since conda main channel usually lacks behind on the latest releases,
# we use pypi to test against the latest releases of the dependencies.
# conda is still used as a convenient way to install Python and pip.
# We let pip install the latest version of the build dependencies from
# pyproject.toml and the runtime dependencies from the dist-info metadata
# that results from the scikit-learn setup.py file.
# The optional test are installed separately:
make_conda "python=$PYTHON_VERSION"
python -m pip install numpy scipy joblib cython
python -m pip install -U pip
python -m pip install pytest==$PYTEST_VERSION pytest-cov pytest-xdist
python -m pip install pandas matplotlib pyamg
fi
Expand Down Expand Up @@ -117,5 +122,14 @@ except ImportError:
print('pandas not installed')
"
python -m pip list
python setup.py build_ext --inplace -j 3
python setup.py develop

if [[ "$DISTRIB" == "conda-pip-latest" ]]; then
# Check that pip can automatically install the build dependencies from
# pyproject.toml using an isolated build environment:
pip install --verbose --editable .
else
# Use the pre-installed build dependencies and build directly in the
# current environment.
python setup.py build_ext --inplace -j 3
python setup.py develop
fi
7 changes: 7 additions & 0 deletions doc/developers/advanced_installation.rst
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,13 @@ Build dependencies

Building Scikit-learn also requires:

..
# The following places need to be in sync with regard to Cython version:
# - pyproject.toml
# - .circleci config file
# - sklearn/_build_utils/__init__.py
# - advanced installation guide

- Cython >= 0.28.5
- A C/C++ compiler and a matching OpenMP_ runtime library. See the
:ref:`platform system specific instructions
Expand Down
15 changes: 15 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
[build-system]
# Minimum requirements for the build system to execute.
requires = [
"setuptools",
"wheel",
"numpy>=1.11",
"scipy>=0.17.0",
# on conda, this is the latest for python 3.5
# The following places need to be in sync with regard to Cython version:
# - pyproject.toml
# - .circleci config file
# - sklearn/_build_utils/__init__.py
# - advanced installation guide
"Cython>=0.28.5",
]
93 changes: 36 additions & 57 deletions sklearn/_build_utils/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,75 +6,54 @@


import os

from distutils.version import LooseVersion
import contextlib

from .openmp_helpers import check_openmp_support


DEFAULT_ROOT = 'sklearn'
# on conda, this is the latest for python 3.5

# The following places need to be in sync with regard to Cython version:
# - pyproject.toml
# - .circleci config file
# - sklearn/_build_utils/__init__.py
# - advanced installation guide
CYTHON_MIN_VERSION = '0.28.5'


def build_from_c_and_cpp_files(extensions):
"""Modify the extensions to build from the .c and .cpp files.

This is useful for releases, this way cython is not required to
run python setup.py install.
"""
for extension in extensions:
sources = []
for sfile in extension.sources:
path, ext = os.path.splitext(sfile)
if ext in ('.pyx', '.py'):
if extension.language == 'c++':
ext = '.cpp'
else:
ext = '.c'
sfile = path + ext
sources.append(sfile)
extension.sources = sources


def maybe_cythonize_extensions(top_path, config):
"""Tweaks for building extensions between release and development mode."""
def cythonize_extensions(top_path, config):
"""Check that a recent Cython is available and cythonize extensions"""
with_openmp = check_openmp_support()

is_release = os.path.exists(os.path.join(top_path, 'PKG-INFO'))

if is_release:
build_from_c_and_cpp_files(config.ext_modules)
else:
message = ('Please install cython with a version >= {0} in order '
'to build a scikit-learn development version.').format(
CYTHON_MIN_VERSION)
try:
import Cython
if LooseVersion(Cython.__version__) < CYTHON_MIN_VERSION:
message += ' Your version of Cython was {0}.'.format(
Cython.__version__)
raise ValueError(message)
from Cython.Build import cythonize
except ImportError as exc:
exc.args += (message,)
raise

n_jobs = 1
with contextlib.suppress(ImportError):
import joblib
if LooseVersion(joblib.__version__) > LooseVersion("0.13.0"):
# earlier joblib versions don't account for CPU affinity
# constraints, and may over-estimate the number of available
# CPU particularly in CI (cf loky#114)
n_jobs = joblib.effective_n_jobs()

config.ext_modules = cythonize(
config.ext_modules,
nthreads=n_jobs,
compile_time_env={'SKLEARN_OPENMP_SUPPORTED': with_openmp},
compiler_directives={'language_level': 3})
message = ('Please install cython with a version >= {0} in order '
'to build a scikit-learn from source.').format(
CYTHON_MIN_VERSION)
try:
import Cython
if LooseVersion(Cython.__version__) < CYTHON_MIN_VERSION:
message += (' The current version of Cython is {} installed in {}.'
.format(Cython.__version__, Cython.__path__))
raise ValueError(message)
from Cython.Build import cythonize
except ImportError as exc:
exc.args += (message,)
raise

n_jobs = 1
with contextlib.suppress(ImportError):
import joblib
if LooseVersion(joblib.__version__) > LooseVersion("0.13.0"):
# earlier joblib versions don't account for CPU affinity
# constraints, and may over-estimate the number of available
# CPU particularly in CI (cf loky#114)
n_jobs = joblib.cpu_count()

config.ext_modules = cythonize(
config.ext_modules,
nthreads=n_jobs,
compile_time_env={'SKLEARN_OPENMP_SUPPORTED': with_openmp},
compiler_directives={'language_level': 3})


def gen_from_templates(templates, top_path):
Expand Down
6 changes: 4 additions & 2 deletions sklearn/setup.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import sys
import os

from sklearn._build_utils import maybe_cythonize_extensions
from sklearn._build_utils import cythonize_extensions
from sklearn._build_utils.deprecated_modules import (
_create_deprecated_modules_files
)
Expand Down Expand Up @@ -78,7 +79,8 @@ def configuration(parent_package='', top_path=None):
# add the test directory
config.add_subpackage('tests')

maybe_cythonize_extensions(top_path, config)
if 'sdist' not in sys.argv:
cythonize_extensions(top_path, config)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not too sure about this as it prevents to do things like python setup.py sdist bdist_wheel in a single line but maybe we don't care. Our CI does not do this.


return config

Expand Down
4 changes: 4 additions & 0 deletions sklearn/tests/test_common.py
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,10 @@ def test_check_estimator_generate_only():
def test_configure():
# Smoke test the 'configure' step of setup, this tests all the
# 'configure' functions in the setup.pys in scikit-learn
# This test requires Cython which is not necessarily there when running
# the tests of an installed version of scikit-learn or when scikit-learn
# is installed in editable mode by pip build isolation enabled.
pytest.importorskip("Cython")
cwd = os.getcwd()
setup_path = os.path.abspath(os.path.join(sklearn.__path__[0], '..'))
setup_filename = os.path.join(setup_path, 'setup.py')
Expand Down