[MRG] Make pip install numpy and scipy for us #10398
Conversation
Remove unnecessary blank line (flake8)
Suppress flake8 error on an import that we do just to see if the library is present
|
See #7867 (comment) |
|
The last build looked good except the Python 2.7 tests hung and were killed. I'm pretty sure this was unrelated to my changes, so I pushed an empty commit to trigger a new build. |
|
Please follow @njsmith recommendation. He is an expert on this packaging issues and he thinks that's the best practice, then that is how we should do it. Can you commit something following his approach and if that does not work as you seemed to say in the related issue, we'll try to figure out why. |
|
I created a separate PR so we can evaluate the other approach: This gives us the flexibility to choose which one to merge and which one to close. |
| # for this platform, so it is safe to add numpy to build requirements. | ||
| build_requires += ( | ||
| ['numpy>={0}'.format(NUMPY_MIN_VERSION)] | ||
| if 'bdist_wheel' in sys.argv[1:] else []) |
There was a problem hiding this comment.
I think this would be clearer as an if statement rather than appending an empty list.
Also, in the case where we add [], should we be checking that the numpy version meets minimum requirements?
|
|
||
| if build_requires: | ||
| metadata.update( | ||
| setup_requires=build_requires, |
There was a problem hiding this comment.
I think @njsmith was saying we should not use this...?
This PR modifies
setup.pyto includenumpyandscipyas setup dependencies. Thus, runningpip install scikit-learnwill automatically install these libraries if needed. This code was adapted fromscipy.Fix #7867.