10000 BLD: Do not pre-cythonize when calling sdist (#15567) · scikit-learn/scikit-learn@82e2f54 · GitHub
[go: up one dir, main page]
Skip to content

Commit 82e2f54

Browse files
ogriseljeremiedbb
ogrisel
authored and
jeremiedbb
committed
BLD: Do not pre-cythonize when calling sdist (#15567)
1 parent 981fa7b commit 82e2f54

File tree

8 files changed

+74
-61
lines changed

8 files changed

+74
-61
lines changed

‎.circleci/config.yml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,11 @@ jobs:
1313
- NUMPY_VERSION: 1.11.0
1414
- SCIPY_VERSION: 0.17.0
1515
- MATPLOTLIB_VERSION: 1.5.1
16+
# on conda, this is the latest for python 3.5
17+
# The following places need to be in sync with regard to Cython version:
18+
# - .circleci config file
19+
# - sklearn/_build_utils/__init__.py
20+
# - advanced installation guide
1621
- CYTHON_VERSION: 0.28.5
1722
- SCIKIT_IMAGE_VERSION: 0.12.3
1823
steps:

‎MANIFEST.in

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,4 +5,3 @@ recursive-include sklearn *.c *.h *.pyx *.pxd *.pxi *.tp
55
recursive-include sklearn/datasets *.csv *.csv.gz *.rst *.jpg *.txt *.arff.gz *.json.gz
66
include COPYING
77
include README.rst
8-

‎build_tools/azure/install.sh

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,8 @@ elif [[ "$DISTRIB" == "conda-pip-latest" ]]; then
9090
# we use pypi to test against the latest releases of the dependencies.
9191
# conda is still used as a convenient way to install Python and pip.
9292
make_conda "python=$PYTHON_VERSION"
93-
python -m pip install numpy scipy joblib cython
93+
python -m pip install -U pip
94+
python -m pip install numpy scipy cython joblib
9495
python -m pip install pytest==$PYTEST_VERSION pytest-cov pytest-xdist
9596
python -m pip install pandas matplotlib pyamg
9697
fi
@@ -118,5 +119,8 @@ except ImportError:
118119
print('pandas not installed')
119120
"
120121
python -m pip list
122+
123+
# Use setup.py instead of `pip install -e .` to be able to pass the -j flag
124+
# to speed-up the building multicore CI machines.
121125
python setup.py build_ext --inplace -j 3
122126
python setup.py develop

‎doc/developers/advanced_installation.rst

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,12 @@ Build dependencies
101101

102102
Building Scikit-learn also requires:
103103

104+
..
105+
# The following places need to be in sync with regard to Cython version:
106+
# - .circleci config file
107+
# - sklearn/_build_utils/__init__.py
108+
# - advanced installation guide
109+
104110
- Cython >= 0.28.5
105111
- A C/C++ compiler and a matching OpenMP_ runtime library. See the
106112
:ref:`platform system specific instructions

‎setup.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,7 @@ def configuration(parent_package='', top_path=None):
161161
os.remove('MANIFEST')
162162

163163
from numpy.distutils.misc_util import Configuration
164+
from sklearn._build_utils import _check_cython_version
164165

165166
config = Configuration(None, parent_package, top_path)
166167

@@ -171,6 +172,12 @@ def configuration(parent_package='', top_path=None):
171172
delegate_options_to_subpackages=True,
172173
quiet=True)
173174

175+
# Cython is required by config.add_subpackage for templated extensions
176+
# that need the tempita sub-submodule. So check that we have the correct
177+
# version of Cython so as to be able to raise a more informative error
178+
# message from the start if it's not the case.
179+
_check_cython_version()
180+
174181
config.add_subpackage('sklearn')
175182

176183
return config
@@ -240,6 +247,7 @@ def setup_package():
240247
len(sys.argv) >= 2 and ('--help' in sys.argv[1:] or
241248
sys.argv[1] in ('--help-commands',
242249
'egg_info',
250+
'dist_info',
243251
'--version',
244252
'clean'))):
245253
# For these actions, NumPy is not required

‎sklearn/_build_utils/__init__.py

Lines changed: 39 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -6,75 +6,57 @@
66

77

88
import os
9-
109
from distutils.version import LooseVersion
1110
import contextlib
1211

1312
from .openmp_helpers import check_openmp_support
1413

1514

1615
DEFAULT_ROOT = 'sklearn'
17-
# on conda, this is the latest for python 3.5
16+
17+
# The following places need to be in sync with regard to Cython version:
18+
# - .circleci config file
19+
# - sklearn/_build_utils/__init__.py
20+
# - advanced installation guide
1821
CYTHON_MIN_VERSION = '0.28.5'
1922

2023

21-
def build_from_c_and_cpp_files(extensions):
22-
"""Modify the extensions to build from the .c and .cpp files.
23-
24-
This is useful for releases, this way cython is not required to
25-
run python setup.py install.
26-
"""
27-
for extension in extensions:
28-
sources = []
29-
for sfile in extension.sources:
30-
path, ext = os.path.splitext(sfile)
31-
if ext in ('.pyx', '.py'):
32-
if extension.language == 'c++':
33-
ext = '.cpp'
34-
else:
35-
ext = '.c'
36-
sfile = path + ext
37-
sources.append(sfile)
38-
extension.sources = sources
39-
40-
41-
def maybe_cythonize_extensions(top_path, config):
42-
"""Tweaks for building extensions between release and development mode."""
43-
with_openmp = check_openmp_support()
24+
def _check_cython_version():
25+
message = ('Please install Cython with a version >= {0} in order '
26+
'to build a scikit-learn from source.').format(
27+
CYTHON_MIN_VERSION)
28+
try:
29+
import Cython
30+
except ModuleNotFoundError:
31+
# Re-raise with more informative error message instead:
32+
raise ModuleNotFoundError(message)
4433

45-
is_release = os.path.exists(os.path.join(top_path, 'PKG-INFO'))
34+
if LooseVersion(Cython.__version__) < CYTHON_MIN_VERSION:
35+
message += (' The current version of Cython is {} installed in {}.'
36+
.format(Cython.__version__, Cython.__path__))
37+
raise ValueError(message)
4638

47-
if is_release:
48-
build_from_c_and_cpp_files(config.ext_modules)
49-
else:
50-
message = ('Please install cython with a version >= {0} in order '
51-
'to build a scikit-learn development version.').format(
52-
CYTHON_MIN_VERSION)
53-
try:
54-
import Cython
55-
if LooseVersion(Cython.__version__) < CYTHON_MIN_VERSION:
56-
message += ' Your version of Cython was {0}.'.format(
57-
Cython.__version__)
58-
raise ValueError(message)
59-
from Cython.Build import cythonize
60-
except ImportError as exc:
61-
exc.args += (message,)
62-
raise
63-
64-
n_jobs = 1
65-
with contextlib.suppress(ImportError):
66-
import joblib
67-
if LooseVersion(joblib.__version__) > LooseVersion("0.13.0"):
68-
# earlier joblib versions don't account for CPU affinity
69-
# constraints, and may over-estimate the number of available
70-
# CPU particularly in CI (cf loky#114)
71-
n_jobs = joblib.effective_n_jobs()
72-
73-
config.ext_modules = cythonize(
74-
config.ext_modules,
75-
nthreads=n_jobs,
76-
compile_time_env={'SKLEARN_OPENMP_SUPPORTED': with_openmp},
77-
compiler_directives={'language_level': 3})
39+
40+
def cythonize_extensions(top_path, config):
41+
"""Check that a recent Cython is available and cythonize extensions"""
42+
_check_cython_version()
43+
from Cython.Build import cythonize
44+
45+
with_openmp = check_openmp_support()
46+
n_jobs = 1
47+
with contextlib.suppress(ImportError):
48+
import joblib
49+
if LooseVersion(joblib.__version__) > LooseVersion("0.13.0"):
50+
# earlier joblib versions don't account for CPU affinity
51+
# constraints, and may over-estimate the number of available
52+
# CPU particularly in CI (cf loky#114)
53+
n_jobs = joblib.cpu_count()
54+
55+
config.ext_modules = cythonize(
56+
config.ext_modules,
57+
nthreads=n_jobs,
58+
compile_time_env={'SKLEARN_OPENMP_SUPPORTED': with_openmp},
59+
compiler_directives={'language_level': 3})
7860

7961

8062
def gen_from_templates(templates, top_path):

‎sklearn/setup.py

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
1+
import sys
12
import os
23

3-
from sklearn._build_utils import maybe_cythonize_extensions
4+
from sklearn._build_utils import cythonize_extensions
45
from sklearn._build_utils.deprecated_modules import (
56
_create_deprecated_modules_files
67
)
@@ -78,7 +79,11 @@ def configuration(parent_package='', top_path=None):
7879
# add the test directory
7980
config.add_subpackage('tests')
8081

81-
maybe_cythonize_extensions(top_path, config)
82+
# Skip cythonization as we do not want to include the generated
83+
# C/C++ files in the release tarballs as they are not necessarily
84+
# forward compatible with future versions of Python for instance.
85+
if 'sdist' not in sys.argv:
86+
cythonize_extensions(top_path, config)
8287

8388
return config
8489

‎sklearn/tests/test_common.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,10 @@ def test_check_estimator_generate_only():
125125
def test_configure():
126126
# Smoke test the 'configure' step of setup, this tests all the
127127
# 'configure' functions in the setup.pys in scikit-learn
128+
# This test requires Cython which is not necessarily there when running
129+
# the tests of an installed version of scikit-learn or when scikit-learn
130+
# is installed in editable mode by pip build isolation enabled.
131+
pytest.importorskip("Cython")
128132
cwd = os.getcwd()
129133
setup_path = os.path.abspath(os.path.join(sklearn.__path__[0], '..'))
130134
setup_filename = os.path.join(setup_path, 'setup.py')

0 commit comments

Comments
 (0)
0