Repository: scikit-learn/scikit-learn
Pull request: [MRG] require scipy in setup
Could fix #15595.

This requires scipy in setup.py, but I'm sure we shouldn't do this. This is a starting point for us to figure out what to do. Feel free to change or open another PR.

Problem statement:
sklearn/utils/_cython_blas.pyx fails to cythonize if scipy is not installed in the env
Now that we don't cythonize before creating the sdist, using the sdist fails to properly install the package if the freshly created environment doesn't include scipy.

`pip install scikit-learnxxx.tar.gz` on an environment which has numpy and cython, will fail while trying to cythonize `sklearn/utils/_cython_blas.pyx`.

Example error:

```
 sklearn/utils/_cython_blas.pyx:203:40: Converting to Python object not allowed without gil
  
  Error compiling Cython file:
  ------------------------------------------------------------
  ...
          if floating is float:
              sgemm(&ta_, &tb_, &m, &n, &k, &alpha, A,
                    &lda, B, &ldb, &beta, C, &ldc)
          else:
              dgemm(&ta_, &tb_, &m, &n, &k, &alpha, A,
                    &lda, B, &ldb, &beta, C, &ldc)
                                            ^
  ------------------------------------------------------------
  
  sklearn/utils/_cython_blas.pyx:203:43: Converting to Python object not allowed without gil

```

Hints from issue discussion:
`dgemm` is cimported from scipy and if cython can't find (scipy isn't installed) it it will assume it's a python thing?

Isn't scipy a build dependency anyway?
the way to specify build dependencies is to use `pyproject.toml`, which we've decided not to include in this release. Should we then error and tell users to install scipy before installing from sdist?

@ogrisel, @glemaitre , WDYT?
although technically, we should only error if the user is installing the package from source, and doesn't have scipy, otherwise they don't need scipy during setup. Not sure how to do that.

Commit message: require scipy in setup

Patch to review:
diff --git a/setup.py b/setup.py
index dc0791f609ca6..00f8587a86ba7 100755
--- a/setup.py
+++ b/setup.py
@@ -11,6 +11,7 @@
 from distutils.command.clean import clean as Clean
 from pkg_resources import parse_version
 import traceback
+import importlib
 try:
     import builtins
 except ImportError:
@@ -183,24 +184,24 @@ def configuration(parent_package='', top_path=None):
     return config
 
 
-def get_numpy_status():
+def get_package_status(package, min_version):
     """
-    Returns a dictionary containing a boolean specifying whether NumPy
+    Returns a dictionary containing a boolean specifying whether given package
     is up-to-date, along with the version string (empty string if
     not installed).
     """
-    numpy_status = {}
+    package_status = {}
     try:
-        import numpy
-        numpy_version = numpy.__version__
-        numpy_status['up_to_date'] = parse_version(
-            numpy_version) >= parse_version(NUMPY_MIN_VERSION)
-        numpy_status['version'] = numpy_version
+        p = importlib.import_module(package)
+        package_version = p.__version__
+        package_status['up_to_date'] = parse_version(
+            package_version) >= parse_version(min_version)
+        package_status['version'] = package_version
     except ImportError:
         traceback.print_exc()
-        numpy_status['up_to_date'] = False
-        numpy_status['version'] = ""
-    return numpy_status
+        package_status['up_to_date'] = False
+        package_status['version'] = ""
+    return package_status
 
 
 def setup_package():
@@ -268,7 +269,7 @@ def setup_package():
                 " Python version is %s installed in %s."
                 % (platform.python_version(), sys.executable))
 
-        numpy_status = get_numpy_status()
+        numpy_status = get_package_status('numpy', NUMPY_MIN_VERSION)
         numpy_req_str = "scikit-learn requires NumPy >= {}.\n".format(
             NUMPY_MIN_VERSION)
 
@@ -287,6 +288,21 @@ def setup_package():
                                   "installed.\n{}{}"
                                   .format(numpy_req_str, instructions))
 
+        scipy_status = get_package_status('scipy', SCIPY_MIN_VERSION)
+        scipy_req_str = "scikit-learn requires scipy >= {}.\n".format(
+            SCIPY_MIN_VERSION)
+
+        if scipy_status['up_to_date'] is False:
+            if scipy_status['version']:
+                raise ImportError("Your installation of "
+                                  "scipy {} is out-of-date.\n{}{}"
+                                  .format(numpy_status['version'],
+                                          numpy_req_str, instructions))
+            else:
+                raise ImportError("scipy is not "
+                                  "installed.\n{}{}"
+                                  .format(scipy_req_str, instructions))
+
         from numpy.distutils.core import setup
 
         metadata['configuration'] = configuration


Code review comments:
diff_hunk: @@ -183,24 +184,24 @@ def configuration(parent_package='', top_path=None):
     return config
 
 
-def get_numpy_status():
+def get_package_status(package, min_version):
     """
-    Returns a dictionary containing a boolean specifying whether NumPy
+    Returns a dictionary containing a boolean specifying whether given package
     is up-to-date, along with the version string (empty string if
     not installed).
     """
-    numpy_status = {}
+    package_status = {}
     try:
-        import numpy
-        numpy_version = numpy.__version__
-        numpy_status['up_to_date'] = parse_version(
-            numpy_version) >= parse_version(NUMPY_MIN_VERSION)
-        numpy_status['version'] = numpy_version
+        p = importlib.import_module(package)
+        package_version = p.__version__
path: setup.py
text: @user1:
```suggestion
        package_version = module.__version__
```

diff_hunk: @@ -183,24 +184,24 @@ def configuration(parent_package='', top_path=None):
     return config
 
 
-def get_numpy_status():
+def get_package_status(package, min_version):
     """
-    Returns a dictionary containing a boolean specifying whether NumPy
+    Returns a dictionary containing a boolean specifying whether given package
     is up-to-date, along with the version string (empty string if
     not installed).
     """
-    numpy_status = {}
+    package_status = {}
     try:
-        import numpy
-        numpy_version = numpy.__version__
-        numpy_status['up_to_date'] = parse_version(
-            numpy_version) >= parse_version(NUMPY_MIN_VERSION)
-        numpy_status['version'] = numpy_version
+        p = importlib.import_module(package)
path: setup.py
text: @user1:
```suggestion
        module = importlib.import_module(package)
```