Declare Numpy as a setup dependency #2445

Merged
merged 4 commits into from Sep 27, 2013

Conversation

Projects
None yet
3 participants
Owner

mdboom commented Sep 20, 2013

This will install Numpy before matplotlib, so that we can get at its headers at compile-time.

Note that this doesn't work with python setup.py install, it only works when you do pip install . (I think that's by design). But this should also fix pip install matplotlib from PyPI, which is awfully nice.

Thanks to @tcaswell, @WeatherGod and @pelson for their thoughts about this on the mailing list, and https://github.com/h5py/h5py/pull/356/files from which a lot of this is shamelessly stolen.

Owner

mdboom commented Sep 20, 2013

BTW: I'm a little on the fence about whether this belongs on 1.3.x, but it would be nice to get it in and fix pip installs sooner rather than later.

Member

WeatherGod commented Sep 20, 2013

I say that it is a bug fix for 1.3.x. The upcoming 1.3.1 release will contain many installation fixes, so it makes perfect sense to include this one with it. I am going to do some testing with this. I am going to see if I can make this break or not in a few different ways.

Member

WeatherGod commented Sep 20, 2013

yeah, that issue with it not working for "python setup.py install" needs to be figured out. The problem is that tools like easy_install that can install from a source distribution basically does that under the hood.

For my own packages at work (much more simple than matplotlib's though), I use setup_requires for things like the coverage package and nose. It seems to always make sure I have them (probably a little too often) but it does seem to work as expected. Perhaps we are being a little too restrictive in when to set setup_requires?

Owner

mdboom commented Sep 20, 2013

Hmm... So in your other projects, setting setup_requires works with python setup.py install? It's not clear to me from the docs that's supposed to work.

I'm not sure how we could be less restrictive in when we set setup_requires. The only time it's turned off is when a --help argument is set.

Member

WeatherGod commented Sep 20, 2013

Yes, so, if I have nose and coverage specified in both setup_requires and install_requires, then by the end of "python setup.py install" in a completely clean environment, I will have nose and coverage eggs both in my local directory (from setup_requires), and in my site-packages (from the install_requires). The error that is happening when calling install for this branch is that include_dirs_hook() is being called prior to any downloads of dependencies, which is strange, because that is being called by the build_ext command for the matplotlib.ft2font extension.

Owner

mdboom commented Sep 20, 2013

Yes, I would have assumed any build-time dependencies would be downloaded before built_ext is run. There's obviously something missing here.

Member

WeatherGod commented Sep 21, 2013

I know they are now identical, but I have run into issues with distribute
and setuptools conflicting with each other. Perhaps we should explicitly
use setuptools?

Perhaps another avenue is to make a fake, simple package that only depends
on numpy and try to get that to work? I find it difficult to wade through
our setup.py and setupext.py.

Member

WeatherGod commented Sep 21, 2013

While researching this a bit further, I came across an interesting feature we might want to use: "extras_require" and "features":

     |   'extras_require' -- a dictionary mapping names of optional "extras" to the
     |      additional requirement(s) that using those extras incurs. For example,
     |      this::
     |  
     |          extras_require = dict(reST = ["docutils>=0.3", "reSTedit"])
     |  
     |      indicates that the distribution can optionally provide an extra
     |      capability called "reST", but it can only be used if docutils and
     |      reSTedit are installed.  If the user installs your package using
     |      EasyInstall and requests one of your extras, the corresponding
     |      additional requirements will be installed if needed.
     |  
     |   'features' -- a dictionary mapping option names to 'setuptools.Feature'
     |      objects.  Features are a portion of the distribution that can be
     |      included or excluded based on user options, inter-feature dependencies,
     |      and availability on the current system.  Excluded features are omitted
     |      from all setup commands, including source and binary distributions, so
     |      you can create multiple distributions from the same source tree.
     |      Feature names should be valid Python identifiers, except that they may
     |      contain the '-' (minus) sign.  Features can be included or excluded
     |      via the command line options '--with-X' and '--without-X', where 'X' is
     |      the name of the feature.  Whether a feature is included by default, and
     |      whether you are allowed to control this from the command line, is
     |      determined by the Feature object.  See the 'Feature' class for more
     |      information.
Member

WeatherGod commented Sep 21, 2013

Another situation where you would want the setup_requires list to be empty besides "--help" is for "clean".

Member

WeatherGod commented Sep 21, 2013

Hmm, strange... in my dead simple setup.py where I have numpy as a setup_requires, simply doing "python setup.py clean" causes numpy to get installed locally. But for this branch, no attempt at downloading numpy happens.

Member

WeatherGod commented Sep 21, 2013

An observation, while fixing this didn't make things work for me (yet), I noticed that the Extension objects and Distribute objects were coming from distutils instead of setuptools. Since setup_requires is a setuptools feature, I wouldn't be surprised that a Distribution from distutils wouldn't recognize it.

Member

pelson commented Sep 23, 2013

👍 for including this in v1.3.1

Owner

mdboom commented Sep 23, 2013

Ok -- I think this is working, I was just testing it wrong.

I have a test script in unit/test_clean_install.sh that demonstrates how this should work. Anyone want to confirm that it works for them. It's really nice to finally resolve this problem.

Member

WeatherGod commented Sep 23, 2013

Haven't tested, but virtualenv.py does not technically work completely by itself. There is virtualenv_support folder that is needed as well.

Should we be using distutils's Distribution or should we be using setuptool's Distribution class? Same question for Extension class in setupext.py

Owner

mdboom commented Sep 23, 2013

I believe I have included the standalone version of virtualenv.py. Let me know if that's not the case.

setuptools monkeypatches distutils, so importing from either place is the same thing, AFAIK.

Member

WeatherGod commented Sep 24, 2013

Seems to work for me, although the messages at the beginning from compiling numpy in the local space is fairly "scary". There was an odd warning towards the end of the install for me:

/tmp/easy_install-CNtSMb/numpy-1.7.1/numpy/distutils/misc_util.py:252: RuntimeWarning: Parent module 'numpy.distutils' not found while handling absolute import

Another possible command to ignore setup_requires might be "sdist".

Member

WeatherGod commented Sep 24, 2013

A possible issue I just came across...

So, let's say that one time the numpy package gets installed at that local location, it then becomes the location to use for compiling matplotlib in any future builds regardless of whether numpy is installed or not, completely ignoring the available numpy for that environment.

This is technically also a similar issue to one where matplotlib gets installed prior to an update of an existing numpy install. The build will be made against the wrong numpy (which this PR wouldn't solve).

mdboom added a commit that referenced this pull request Sep 27, 2013

@mdboom mdboom merged commit 87679af into matplotlib:v1.3.x Sep 27, 2013

1 check failed

default The Travis CI build failed
Details
Member

WeatherGod commented Sep 27, 2013

Why was this merged? I think the issue I reported with having an install of numpy sitting locally needs to be discussed. This causes particular havoc when switching between different virtualenvs with different numpys in them because the local install takes precedence.

Owner

mdboom commented Sep 27, 2013

@WeatherGod: Sorry, I thought I had commented on this, but the comment apparently didn't stick.

I think the problem with things being left in the build tree that impact upgrading the dependencies is much larger than this. It happens even without this -- if you build with a particular version of numpy and then build for another virtualenv with another version of numpy in it, you still run into this problem. The only good procedure when moving between versions of numpy is to git clean -fxd anyway. I feel there is enough good here to justify it, and the issue you raise is not really new, unless I'm missing something.

Owner

mdboom commented Sep 27, 2013

Sorry for this, by the way -- I didn't mean to ignore your feedback. I really thought I had commented on it and I interpreted your lack of reply as agreement. It turns out I think the comment never posted (probably because I forgot to hit the "Comment" button). If there's strong disagreement about this, I'm happy to revert it before cutting 1.3.1.

Member

WeatherGod commented Sep 27, 2013

Fair enough. And indeed, this isn't exactly anything new, it is just exacerbated, I think, with this PR. Perhaps that would be a good thing? We will need to continue making it clear that "git clean -fxd" should always be done.

@mdboom mdboom deleted the mdboom:numpy-setup-dependency branch Aug 7, 2014

@jzaremba jzaremba referenced this pull request in losonczylab/sima May 21, 2015

Closed

Improve 'pip install sima' functionality #175

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