-
-
Notifications
You must be signed in to change notification settings - Fork 25.8k
CI [scipy-dev] now uses python 3.7 #11409
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
Conversation
Maybe it could be better to add python 3.7 as another build to the Travis build matrix? Since scipy-dev is a cron, it's less systematically checked. I don't think build time would increase that much with another build as Travis appears to use multiple parallel workers... |
Ah, I'd not actually realised it left RC already.
Travis doesn't yet have Python 3.7 natively available, as far as I
understand from the related SciPy issues.
|
Yes, but we could very well install Python 3.7 with conda then numpy & scipy with pip as you doing there. Essentially it could be just copying the |
As long as we can use conda packages for all dependencies, then fine...
otherwise the run will be a lot slower than the others.
|
Why? The scipy/numpy manylinux wheels with OpenBlas would be comparable in speed, aren't they? In my experience in other projects I was originally biased toward using conda, but it turns out that due to slow dependency resolution in conda + downloading/unpacking mkl which is huge, it's often faster to just use wheels from PyPi, if test runtime is comparable. |
Well I'm not entirely certain why, but (here)[https://travis-ci.org/scikit-learn/scikit-learn/builds/399462496?utm_source=github_status&utm_medium=notification] it takes 10 more minutes to build for 3.7. |
Running
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We would need to run the test Python 3.7 under windows on appveyor before claiming Python 3.7 support.
But I am fine merging this PR as it is (besides minor comment).
build_tools/travis/install.sh
Outdated
@@ -82,8 +87,8 @@ elif [[ "$DISTRIB" == "scipy-dev" ]]; then | |||
# Set up our own virtualenv environment to avoid travis' numpy. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/virtualenv/conda env/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we end up using conda to have Python 3.7 I think we can get rid of this comment ("Set up our own virtualenv ...") and the one below:
# This venv points to the python interpreter of the travis build
# matrix.
Actually, the test runtime is an issue but it should improve quickly as soon as the binary packages get uploaded. |
They are already uploaded for stable numpy and scipy . The reason this build takes so long ,I think, is because it uses scipy-dev (i.e. numpy dev wheels aren't available for Py3.7 yet from what I gather). So having one separate build for scipy-dev (cron) and one for Py3.7 (non cron) using stable numpy and scipy would be a way around it.. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have not realized that Py3.7 is tested in Appveyor already. Merged master in and removed "virtualenv" related comments as suggested by @lesteve .
This LGTM, will merge when CI is green (unless someone has other comments).
# matrix. | ||
virtualenv --python=python ~/testvenv | ||
source ~/testvenv/bin/activate | ||
make_conda python=3.7 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I now wonder if we should just use python
here without pinning the version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Anticipating Python 4 ? )
I think it doesn't hurt to indicate explicitly for the reader that it tests 3.7 ..
(merged master to resolve merge conflict) |
CI passed except for the PyPy3 build which is now temporary disabled. Merging, thanks @jnothman ! |
Fixing #11320...?