scikit-learn · jeremiedbb · Nov 8, 2019 · Nov 4, 2019 · Nov 8, 2019 · Nov 8, 2019
diff --git a/.circleci/config.yml b/.circleci/config.yml
@@ -13,6 +13,11 @@ 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:
+      # - .circleci config file
+      # - sklearn/_build_utils/__init__.py
+      # - advanced installation guide
       - CYTHON_VERSION: 0.28.5
       - SCIKIT_IMAGE_VERSION: 0.12.3
     steps:

diff --git a/MANIFEST.in b/MANIFEST.in
@@ -5,4 +5,3 @@ 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
-
diff --git a/build_tools/azure/install.sh b/build_tools/azure/install.sh
@@ -90,7 +90,8 @@ elif [[ "$DISTRIB" == "conda-pip-latest" ]]; then
     # 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.
     make_conda "python=$PYTHON_VERSION"
-    python -m pip install numpy scipy joblib cython
+    python -m pip install -U pip
+    python -m pip install numpy scipy cython joblib
     python -m pip install pytest==$PYTEST_VERSION pytest-cov pytest-xdist
     python -m pip install pandas matplotlib pyamg
 fi
@@ -118,5 +119,8 @@ except ImportError:
     print('pandas not installed')
 "
 python -m pip list
+
+# Use setup.py instead of `pip install -e .` to be able to pass the -j flag
+# to speed-up the building multicore CI machines.
 python setup.py build_ext --inplace -j 3
 python setup.py develop
diff --git a/doc/developers/advanced_installation.rst b/doc/developers/advanced_installation.rst
@@ -101,6 +101,12 @@ Build dependencies
 
 Building Scikit-learn also requires:
 
+..
+    # The following places need to be in sync with regard to Cython version:
+    # - .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

diff --git a/setup.py b/setup.py
@@ -161,6 +161,7 @@ def configuration(parent_package='', top_path=None):
         os.remove('MANIFEST')
 
     from numpy.distutils.misc_util import Configuration
+    from sklearn._build_utils import _check_cython_version
 
     config = Configuration(None, parent_package, top_path)
 
@@ -171,6 +172,12 @@ def configuration(parent_package='', top_path=None):
                        delegate_options_to_subpackages=True,
                        quiet=True)
 
+    # Cython is required by config.add_subpackage for templated extensions
+    # that need the tempita sub-submodule. So check that we have the correct
+    # version of Cython so as to be able to raise a more informative error
+    # message from the start if it's not the case.
+    _check_cython_version()
+
     config.add_subpackage('sklearn')
 
     return config
@@ -240,6 +247,7 @@ def setup_package():
             len(sys.argv) >= 2 and ('--help' in sys.argv[1:] or
                                     sys.argv[1] in ('--help-commands',
                                                     'egg_info',
+                                                    'dist_info',
                                                     '--version',
                                                     'clean'))):
         # For these actions, NumPy is not required

diff --git a/sklearn/_build_utils/__init__.py b/sklearn/_build_utils/__init__.py
@@ -6,75 +6,57 @@
 
 
 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:
+# - .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."""
-    with_openmp = check_openmp_support()
+def _check_cython_version():
+    message = ('Please install Cython with a version >= {0} in order '
+               'to build a scikit-learn from source.').format(
+                    CYTHON_MIN_VERSION)
+    try:
+        import Cython
+    except ModuleNotFoundError:
+        # Re-raise with more informative error message instead:
+        raise ModuleNotFoundError(message)
 
-    is_release = os.path.exists(os.path.join(top_path, 'PKG-INFO'))
+    if LooseVersion(Cython.__version__) < CYTHON_MIN_VERSION:
+        message += (' The current version of Cython is {} installed in {}.'
+                    .format(Cython.__version__, Cython.__path__))
+        raise ValueError(message)
 
-    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})
+
+def cythonize_extensions(top_path, config):
+    """Check that a recent Cython is available and cythonize extensions"""
+    _check_cython_version()
+    from Cython.Build import cythonize
+
+    with_openmp = check_openmp_support()
+    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):

diff --git a/sklearn/setup.py b/sklearn/setup.py
@@ -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
 )
@@ -78,7 +79,11 @@ def configuration(parent_package='', top_path=None):
     # add the test directory
     config.add_subpackage('tests')
 
-    maybe_cythonize_extensions(top_path, config)
+    # Skip cythonization as we do not want to include the generated
+    # C/C++ files in the release tarballs as they are not necessarily
+    # forward compatible with future versions of Python for instance.
+    if 'sdist' not in sys.argv:
+        cythonize_extensions(top_path, config)
 
     return config
 

diff --git a/sklearn/tests/test_common.py b/sklearn/tests/test_common.py
@@ -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')
Original file line number	Diff line number	Diff line change
Expand Up		@@ -5,4 +5,3 @@ 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