Skip to content
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

setuptools setup_requires installs numpy/cython in unwanted locations #798

Closed
mnmelo opened this issue Mar 23, 2016 · 4 comments
Closed

Comments

@mnmelo
Copy link
Member

mnmelo commented Mar 23, 2016

I'm opening this more as a need to document these setuptools problems, so that the point of the corresponding PR is clearer.

tl;dr:

  • We have coerced setuptools to lazily install build-time dependencies at setup-time, but it does a terrible job at it;
  • Moving dependency installation to build-time would make it cleaner for pip installs, but unfortunately biopython's installer won't like it.

Since PR #499 we've been relatively independent of having numpy preinstalled. #768 suggests we conditionally add Cython to the same lazy dependency scheme.

That lazy dependency workaround makes use of setuptools' setup_requires list. Unfortunately, setup_requires is more of a setuptools' kludge (don't get me started on the unholy mess the whole thing is...). The main downside is that whatever setuptools downloads to satisfy setup-time dependencies ends up being installed in the current dir (!!). The system-wide easy-install.pth even gets a link to that current dir. This is dirty and insecure. (see setuptools issues 209 and 391; it's been dragging on for ages, one of the arguments being that people might rely by now on wrong behavior.)

This problem also leads to issues if I install from source (./setup.py install) while not having numpy:

  • numpy gets downloaded and installed under the source dir;
  • MDAnalysis gets built and installed system-wide;
  • I delete the source dir;
  • import MDAnalysis fails with an ImportError because it can't find numpy.

I'm addressing this together with #768 in an upcoming PR by customizing the dependency installation.

I'd like to move dependency installation to build-time, so that pip install . doesn't even trigger that dependency workaround, which depends too much on setuptools for my liking (in that setting, pip sees MDAnalysis needs numpy and by the time we get to build MDAnalysis that dependency is already met). However, biopython isn't as clever as us, and merely listing its dependencies requires numpy. Therefore I'm having MDAnalysis look for and install build-time dependencies as soon as it detects its being probed by pip (dependency installation is done directly via setuptools, even if in the process of a pip install). This way, by the time we get to probing biopython all is in place (if we get rid of biopython, see #777, then even better!).

@mnmelo mnmelo self-assigned this Mar 23, 2016
@mnmelo mnmelo added this to the 0.15.0 milestone Mar 23, 2016
@kain88-de
Copy link
Member

if we get rid of biopython, see #777, then even better!)

We still need it's KDTree implementation.

@mnmelo
Copy link
Member Author

mnmelo commented Mar 23, 2016

Aww, chucks...

@mnmelo mnmelo mentioned this issue Mar 23, 2016
mnmelo added a commit that referenced this issue Mar 23, 2016
Customized setup-time dependency installation to work around setuptools'
problematic behavior (closes #798)

Modified lazy dependency scheme to be less hacky (we now subclass
distutils.Distribution).

Removed use of Cython's build_ext (redundant with cythonize):
https://groups.google.com/forum/#!topic/cython-users/fBWLUSJWod0
mnmelo added a commit that referenced this issue Mar 23, 2016
Customized setup-time dependency installation to work around setuptools'
problematic behavior (closes #798)

Modified lazy dependency scheme to be less hacky (we now subclass
distutils.Distribution).

Removed use of Cython's build_ext (redundant with cythonize):
https://groups.google.com/forum/#!topic/cython-users/fBWLUSJWod0
@orbeckst
Copy link
Member

Maybe we can eventually replace the KD-Tree with a grid-based search (maybe even multi-threaded and/or GPU accelerated backends?) which would also be better at handling PBC (would make #137 superfluous). Together with #777 (BioPython PDB handling) we could then keep it purely optional (only some minor things like AtomGroup.sequence() might use it)

@kain88-de kain88-de modified the milestones: 0.16.0, 0.15.0 May 7, 2016
@kain88-de
Copy link
Member

I'll close this now. The discussions in #799 lead to the conclusion that all the extra code which depends on internal behaviour of setuptools that might change isn't worth the hand holding for users if we can just give them conda packages.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants