Update install instructions, simplify requirements #1526
Conversation
| install_requires=INSTALL_REQUIRES, | ||
| requires=REQUIRES, |
There was a problem hiding this comment.
This is used to populate the PyPI entry, which does not pick up install_requires.
There was a problem hiding this comment.
I'll put it back. Should it hold both runtime and compile dependencies?
There was a problem hiding this comment.
But please also add a comment since apparently this was a point of confusion. 😄
There was a problem hiding this comment.
It was already commented, but I was to tired to catch it!
|
@blink1073 It's a bit havoc in the code at the time, I'll clean it up when things are starting to look good. Are the |
|
Correct. |
|
Ok, then I will add instructions for those getting scikit-image as source package, and fail more gracefully with instruction for installing scipy, numpy and cython. |
|
Anyone have an idea why the appveyor build does not seem to link cython files correctly? |
|
You took Cython out of requirements.txt, try adding a specific |
| sed -i 's/networkx>=/networkx==/g' requirements.txt | ||
| sed -i '/pillow/d' requirements.txt | ||
| else | ||
| virtualenv -p python --system-site-packages ~/venv |
There was a problem hiding this comment.
This was needed to make the plot examples work. Right now they're timing out in the 2.7 build.
There was a problem hiding this comment.
Do you know what was the magic sauce, venv, pillow, scipy or matplotlib?
System pillow and scipy are not meeting the requirements, should we lower the requirements? If the tests pass, I guess this means we require pillow>=1.1.7 and scipy>=0.9.0.
There was a problem hiding this comment.
There's something in the system site packages (maybe Tk) that was needed to make the matplotlib Template backend work without hanging.
There was a problem hiding this comment.
Agreed on the version changes.
|
Ok, so this is the purposed changes. (I'll clean up the commit noise and Silvester's suggestions later). Note that I'm talking about fresh installs here, neither having runtime or build requirements met. Changes in skimage
The effect is that when scikit-image is installed from binaries (read wheel, that is OS X as of now) and setup.py is not ran, dependencies will be correctly installed. For the source package, it's not the same story, as it will always run setup.py (and fail), never getting to Changes in both test environments
Changes in travis
Changes to appveyor
|
|
Unable to find out when six.moves.urllib was added to six (its above 1.3). Found this: https://bitbucket.org/gutworth/six/pull-request/5/create-sixmovesurllib/diff Anyone familiar with gitbucket? |
|
Seems to be 1.4.0 |
|
@arve0, the reason we want to use our own venv as opposed to the default Travis one is that it kept changing, and breaking our builds. By rolling our own we insulate ourselves from these changes. |
|
Aha. I saw that numpy was already installed in the travis env. I'll use venv for all python version then. |
|
@jjhelmus Hi! I see you are the creator of the appveyor build, can you help me out? Any idea why the build and wheel install is succeeding but the cython import fails? https://ci.appveyor.com/project/stefanv/scikit-image-1fxlb/build/1.0.366 Some excerpts: 707 794 1323 1938 2247 2336 |
|
Seems like files in Edit. No, that's not it: |
|
Ah, found it at last! Cython is actually compiled from source at appveyor, not using version 20, as you've already noted @blink1073. |
|
Great progress so far @arve0, thanks for persevering. |
|
Heh, what a mess! Think I've found the silver bullet. There have been some trouble with travis python 2.7 intervening with system python 2.7. To be specific, travis python have compiled |
|
Btw, do we want to test for PIL (python-imaging) also? If so, I will have to find up some cleverness for hindering the pillow wheel install. |
|
Yes, we explicitly support and tested against PIL. — On Sun, May 24, 2015 at 5:18 PM, Arve Seljebu notifications@github.com
|
| else | ||
| virtualenv -p python --system-site-packages ~/venv | ||
| virtualenv -p $(which python) ~/venv |
There was a problem hiding this comment.
I think the $(which python) is what is breaking your build, try reverting this line.
- cython is not a runtime dependency - dask is a runtime dependency - scikit-image depends on six.moves.urllib
- cython added to setup_requirements - setuptools will call easy_install to install it - all other build requirements, excluding numpy, will be resolved by pip - do not parse requirements.txt - requirements should be the real runtime requirements - fail gracefully when missing numpy - scipy will be resolved by pip from install_requires
arve0
force-pushed
the
installation_instructions
branch
from
f0fac87
to
f51be32
Compare
|
I'm leaving it with this. I'm confident my changes are sound and what remains are test environment issues. I've cleaned up the commit noise, the total should be: Changes in skimage
For the source installs, it's not the same story as with wheels, as it will run setup.py (and fail), never getting to install_requires. Though, if pip are able to run setup.py, it can resolve runtime dependencies (which are overlapping with build dependencies) and build dependencies (setup_requires, a setuptools easy_install directive). So we put build dependencies which do not overlap with runtime dependencies (cython), in setup_requires. Changes in travis
Changes to appveyor
|
- cython 0.20 in wheel house is below scikit-image requirement (>=0.21)
- cython 0.21 will be installed anyhow, save time not installing
cython twice
- dask[arrary] is added to /requirements.txt, and should not be needed
here
requirements.txt should hold all dependencies all deps are not in wheel house - first install binaries from wheel house - then install the rest of the deps
|
2.7 segfaults (numpy 1.6.0 not supported)? https://travis-ci.org/arve0/scikit-image/jobs/64372909 |
|
@arve0, I just built numpy 1.6.1 for the wheelhouse. Try this fix for the intermittent 3.2 error: diff --git a/skimage/io/tests/test_pil.py b/skimage/io/tests/test_pil.py
index cd4e893..07e558e 100644
--- a/skimage/io/tests/test_pil.py
+++ b/skimage/io/tests/test_pil.py
@@ -230,9 +230,9 @@ class TestSaveTIF:
def roundtrip(self, dtype, x, compress):
with temporary_file(suffix='.tif') as fname:
if dtype == np.bool:
- expected = ['low contrast']
+ expected = ['low contrast|unclosed file']
else:
- expected = []
+ expected = ['unclosed file|\A\Z']
with expected_warnings(expected):
if compress > 0:
imsave(fname, x, compress=compress) |
|
@arve0, mind if I stop the previous builds in favor of the latest? |
|
@blink1073 Go ahead. Made me think that building all PRs might not be a good idea, but I'm uncertain if its possible to build PRs "on command"? |
|
Looks like it's gonna pass now: https://travis-ci.org/arve0/scikit-image/builds/64385969 A though: A binstar organization channel might be a good fit for official windows install. What do you think? |
|
@arve0, I'll let @matthew-brett weight in on binstar vs. rackspace. |
|
I canceled the builds, and yes, they can be re-run at any time. |
|
@arve0, what happened to your updated install instructions? |
|
@blink1073, @matthew-brett: Just to be clear and explicit, what I'm thinking of is for Windows users of scikit-image (which are not using a python distribution), not the test system.
Not sure what happened to the updated install instructions, I'll try to find them. |
arve0
force-pushed
the
installation_instructions
branch
from
51f367e
to
8952154
Compare
|
Found the updated install instructions, thank god (Linus) for git. Cleaned up some commits. |
|
You may want to stop the test in favor for #1530, as the test has already passed here: https://travis-ci.org/arve0/scikit-image |
|
@Arve The binstar command looks fine, but would look much better if we can figure out how to build our own windows packages. This is taking forever to debug; I have a branch on which I am trying out Drone, which seems to be more responsive (at least until they reach the same level of success as Travis-CI, I presume) |
|
@stefanv, I love everything about drone.io except for not having parallel builds. |
|
@stefanv, we could be a bit smarter about our builds. We could do one "extra" thing per build: docs, gallery, app gallery, optional dependecies. — On Thu, May 28, 2015 at 5:15 PM, Stefan van der Walt
|
|
Thanks for sticking this out, @arve0! I'm merging this, as it is blocking the dask[array] parallel PR and should fix the intermittent unclosed file error. Any other tweaks, improvements, etc. can happen in separate PRs. |
Add dask[array], improve CI builds, update install instructions
|
Well done @arve0, I owe you a beverage of choice! |
|
More likely I owe you guys a beverage of choice for keeping scikit-image up to such a high quality! |
| Pre-built installation | ||
| Installing scikit-image | ||
| ----------------------- | ||
| If you are on Mac OS X you're lucky, open the terminal and install |
There was a problem hiding this comment.
Why the comment about people on OSX being lucky? I'd get rid of this.
There was a problem hiding this comment.
Surely you did not forget that OSX is the world's most advanced operating system?
And that the latest MacBook is also available in gold?
There was a problem hiding this comment.
Because wheels exists and installation is straight forward containing only one step.
There was a problem hiding this comment.
But joking aside, in this particular case, if you are on OSX, you are lucky. Not a value judgement about operating systems, just a comment on the convenience of pip installs.
There was a problem hiding this comment.
|
@JDWarner This was a premature merge on the docs front. |
|
@stefanv I was going to address that in a separate PR as the technical work was done here, blocking other PRs, and discussions about wording can get lengthy. Didn't see your comments until I submitted #1533 just now, but I think I've addressed most of them. If you prefer to be the PR owner, feel free to close #1533. |
|
See #1535 |
This tries to simplify the requirements and also updates install instructions. It will most probably not work out of the box. Fixes #1506 (when all errors are ironed out).
I'm open to suggestions!