Skip to content

Conversation

@larsoner
Copy link
Contributor

@larsoner larsoner commented Oct 3, 2017

When trying to set up an Anaconda environment using pip to simultaneously install mayavi and PySurfer, I get these errors:

Installing collected packages: mne, nibabel, imageio, pysurfer, nitime, nilearn, quantities, neo, termcolor, pytest-sugar, pytest-faulthandler, pydocstyle, sphinx-bootstrap-theme, traits, pyface, traitsui, configobj, apptools, mayavi, sphinx-gallery
  Running setup.py install for pysurfer ... error
    Complete output from command /home/larsoner/anaconda2/envs/mne/bin/python -u -c "import setuptools, tokenize;__file__='/tmp/pip-build-45nhe837/pysurfer/setup.py';f=getattr(tokenize, 'open', open)(__file__);code=f.read().replace('\r\n', '\n');f.close();exec(compile(code, __file__, 'exec'))" install --record /tmp/pip-3smloy5u-record/install-record.txt --single-version-externally-managed --compile:
    Traceback (most recent call last):
      File "<string>", line 1, in <module>
      File "/tmp/pip-build-45nhe837/pysurfer/setup.py", line 64, in <module>
        check_dependencies()
      File "/tmp/pip-build-45nhe837/pysurfer/setup.py", line 51, in check_dependencies
        raise ImportError("Missing dependencies: %s" % missing)
    ImportError: Missing dependencies: mayavi

This is because we don't tell pip about the mayavi dependency, so it tries to install PySurfer first. We should either remove our custom dependency check, or add mayavi to the deps list -- this PR does the latter.

@mwaskom
Copy link
Member

mwaskom commented Oct 3, 2017

Well, originally this was somewhat on purpose because we assumed most people probably didn't want pip trying to install mayavi. It used to be a operation with a low probability of success. Maybe with more modern python packaging infrastructure it is likely to work better?

@mwaskom
Copy link
Member

mwaskom commented Oct 3, 2017

Oh I see, the issue is that you are trying to pip install mayavi and pysurfer at the same time.

The middle ground is to limit the setup modes in which the dependency check is run: statsmodels/statsmodels@f184b8b

@larsoner
Copy link
Contributor Author

larsoner commented Oct 3, 2017

Maybe with more modern python packaging infrastructure it is likely to work better?

It seems to work decently with Anaconda at least. I definitely don't want to add numpy to the list of dependencies, since that's still asking for trouble.

I think if we eliminate doing dependency checks for install as the mode (which is what I assume pip uses? or something similar that ends up being functionally equivalent?) there isn't much point in having a dependency check in there -- I'm not sure what modes we'd end up wanting to leave it on for.

Either solution is fine by me, though.

@mwaskom
Copy link
Member

mwaskom commented Oct 3, 2017

I think if we eliminate doing dependency checks for install as the mode (which is what I assume pip uses? or something similar that ends up being functionally equivalent?) there isn't much point in having a dependency check in there -- I'm not sure what modes we'd end up wanting to leave it on for.

The way that statsmodels code works is that if you don't have mayavi installed then pip install pysurfer will fail but pip install pysurfer mayavi will work.

@larsoner
Copy link
Contributor Author

larsoner commented Oct 3, 2017

It looks like we already do this check:

https://github.com/nipy/PySurfer/blob/master/setup.py#L59

Does this mean it's not working? Or is the important change elsewhere?

@mwaskom
Copy link
Member

mwaskom commented Oct 3, 2017

It's certainly possible that my memory is failing on aspects of this issue. There was a lot of back and forth about it in seaborn. But honestly, pip is pretty good these days ... I just made a clean conda environment with nothing but python and pip and pip installed numpy/scipy/matplotlib/pandas/seaborn with no issues whatsoever. The default pip install -U behavior remains irritating, but at least they added --no-deps which I tend to do by default. All of this is to argue in favor just just declaring all dependencies in install_requires and washing our hands of the more complicated and less flexible homebrew dependency checking.

@mwaskom
Copy link
Member

mwaskom commented Oct 3, 2017

pip installed numpy/scipy/matplotlib/pandas/seaborn with no issues whatsoever

Forgot to say: on OSX!

@larsoner
Copy link
Contributor Author

larsoner commented Oct 3, 2017

Wow that's pretty good. I was wondering when they'd add something like --no-deps, glad it's in!

So adding mayavi in the list is okay with you? I'm still too afraid to add numpy and scipy because their compiling configurations are harder.

@mwaskom
Copy link
Member

mwaskom commented Oct 3, 2017

With an up-to-date pip I don't think you should need to compile numpy or scipy, pip is pull down binary wheels for me. I'm not sure what the situation is on windows though.

@larsoner
Copy link
Contributor Author

larsoner commented Oct 3, 2017

Okay with you just to add mayavi for now (current PR)?

@mwaskom
Copy link
Member

mwaskom commented Oct 3, 2017

I'm mildly unsatisfied with the piecemeal approach to dependency specification but I see the argument that it assumes the existence of a full Anaconda distribution.

This means mayavi should also come out of the needed_deps list in check_dependencies() right?

@larsoner
Copy link
Contributor Author

larsoner commented Oct 4, 2017

This means mayavi should also come out of the needed_deps list in check_dependencies() right?

Good call, removed (and comment added that it and nibabel are in install_requires).

@mwaskom mwaskom merged commit 1e32519 into nipy:master Oct 4, 2017
@larsoner larsoner deleted the setup branch October 4, 2017 02:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants