Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork8.2k
Retool the setup.py infrastructure#1454
Conversation
mdboom commentedNov 6, 2012
Sorry this is so hard to review. Most of the "639" changed files are just deleted dateutil and pytz. Github used to have a summary of files at the top, but they don't seem to anymore. The interesting files (obviously) are |
WeatherGod commentedNov 6, 2012
Just click "Show diff stats" in the Files Changed tab. |
mdboom commentedNov 6, 2012
Aha! Thanks. |
mdboom commentedNov 16, 2012
No comments on this? If not, I'd like to go ahead and merge this so it gets some testing in the wild. |
mdboom commentedNov 16, 2012
As Mathew Topper pointed out on the mailing list, installing with |
WeatherGod commentedNov 16, 2012
One thing that has always bothered me, and I haven't checked to see if it is still the case, is that any call to setup.py, with the exception of a "--help" option, makes an attempt to check for and/or build some of the code. So, if I am trying to call "python setup.py clean", it tries to build stuff first, or would fail if some things can't be found. |
mdboom commentedNov 16, 2012
@WeatherGod: I don't think that's any longer the case, before or after this patch. |
cgohlke commentedNov 16, 2012
This doesn't work for me on Windows: 1) |
ivanov commentedNov 17, 2012
running setup.py install --user on this branch went out, fetched and installed |
sandrotosi commentedNov 17, 2012
Hi Michael, Can you point me to the changes that hadn't merge upstream for Agg and PyCXX? at least I can try to push them on Debian packages (so others may benefit from them) and then up until upstream. Even clarify what the base versions of those 2 softwares are might be enough, I'll extract the patches myself. |
mdboom commentedNov 19, 2012
@sandrotosi : Not too late. This is a complex patch and it will be a while to get everything verified. The patch required to PyCXX is here: https://github.com/matplotlib/matplotlib/pull/493/files The patch to Agg is not in any sort of state that itcould be permitted upstream -- it involves throwing a PyCXX exception from within Agg. I'll spend a little time seeing if that can be addressed in a better way that would be acceptable upstream -- or even acceptable as a Debian patch. @ivanov: It shouldn't have installed distribute -- only fetched a local copy to use during the matplotlib install (if I understand correctly how all this stuff is supposed to work). That's so if dateutil and pyparsing aren't present, they can be automatically downloaded by setuptools. @cgohlke: Do you have any instructions about how you set up your Windows build environment? Would you be interested in adding such to the docs? It looks like I'm going to need to fire up a virtual machine and get this going. |
mdboom commentedNov 19, 2012
Note also that there should be a new release of pyparsing coming out shortly. |
mdboom commentedNov 19, 2012
@sandrotosi: I have made a better patch against Agg 2.4 here: https://gist.github.com/4112950 We can submit it upstream, or at least consider it for inclusion in Debian's Agg package. This requires a corresponding patch to matplotlib's Note that we don't support building against Agg 2.5 because it is GPL'd (not LGPL'd). (We actually patch a few other things, but they are mainly for broader compiler support -- this is the only one that is critical). |
mdboom commentedNov 19, 2012
@sandrotosi: Also to clarify: the PyCXX we include is based on 6.2.3. I just noticed there is a 6.2.4 (not on the website, but in the downloads list on Sourceforge). Unfortunately, it doesn't address the lack of Python 2.7 support for |
mdboom commentedNov 20, 2012
@cgohlke: I now have the build "completing" on Windows. Would you mind conforming in your binary-building infrastructure when you have a chance? |
mdboom commentedNov 26, 2012
@sandrotosi: An update on the PyCXX front. The lack of PyCapsule support for Python 2.7 is not a problem -- I was building things incorrectly leading me to believe that erroneously. The obstacle now is the lack of buffer support for Python 3.x. I'll whip up a patch for that and send it upstream to Barry Scott (maintainer of PyCXX). I'll also update our local copy to 6.2.4 + patches to work with Python 3.x, since that has a few minor improvements vs. what we have now. |
mdboom commentedDec 6, 2012
@cgohlke,@sandrotosi: Let me know when you've had a chance to look at these recent changes. I'd like to merge this soon in the development cycle to give it lots of testing -- while not throwing something that breaks for a lot of people either ;) |
dmcdougall commentedDec 6, 2012
cgohlke commentedDec 6, 2012
This revision fails to find png.h and tk.h (mpl 1.2 finds them). |
mdboom commentedDec 7, 2012
@cgohlke: On windows, it looks in |
cgohlke commentedDec 7, 2012
@mdboom: Two issues: I don't use the hard coded |
mdboom commentedDec 7, 2012
@cgohlke: Thanks -- that patch is an improvement over all. I think it probably also makes sense to turn |
cgohlke commentedDec 7, 2012
I'm not sure turning on pkgconfig for everything on Windows is a good idea. At least on my system, GTK and associates (PyGTK, PyGObject, PyCairo) are the only packages supporting pkgconfig. If pkgconfig is always on, freetype2 and libpng headers and libraries will be picked up from the GTK directories, but these libraries are outdated and inferior to my libpng and freetype2 builds, which are found through INCLUDE and LIB environment variables. |
mdboom commentedDec 17, 2012
@cgohlke: In your patch above, why is |
cgohlke commentedDec 17, 2012
@mdboom: On my system matplotlib's setupext.py does not find some header files and cancels, unless the INCLUDE paths are added to the |
mdboom commentedDec 17, 2012
@cgohlke: Ok. Can you test this branch again? I think I've integrated what you've proposed (in a slightly different way) by handling the 'INCLUDE' environment variable for everything. |
cgohlke commentedDec 18, 2012
This does not work. |
mdboom commentedDec 19, 2012
My concern with the original patch is that it special cases Gtk. If we're looking for headers in How is Do you have instructions on setting up the build environment you are using? I have this working myself on Windows, but I'm sure my environment is not set up in the same way as yours, and there may be ways in which the binaries I'm producing are not as good... But unless I recreate your environment, I have to back-and-forth things with you like this, obviously. |
pelson commentedFeb 26, 2013
@mdboom - Are all of the changes necessary at the same time? For instance, can PyCXX be updated in a separate PR? Can we remove Pytz & dateutil in a separate PR? There are changes here which surprise me a little (changes to some python 3 I'm all in favour of the re-tool, it's just impossible to see the wood from the trees as it stands. The only other feasible way to move this PR forward without reducing the size of the change, AFAICS, is to hit merge and see what happens... |
mdboom commentedFeb 26, 2013
The changes for review are almost entirely in Unfortunately, it's hard to separate these things out, because once you say "we support building with external libraries", you have to make the changes so that our code compiles with vanilla copies of those libraries (hence the changes to The only changes to Py3 ifdef blocks that I see are within PyCXX, which is simply an updating to the current upstream version -- we shouldn't need to review those changes for style, etc., only confirm that they work by building, running our test suite etc. About the only thing that could be reduced here, I think, is removing dateutil and pytz later -- but again, I think ideally that removal should all be tested together. It's important to know that after removing those files from our repo that matplotlib still installs and works. |
pelson commentedFeb 26, 2013
Ok. Thanks@mdboom . FWIW this works well on OSX 10.8 (Mountain Lion) for python2.7.
On the basis of your last post, I'd suggest we merge this and fix any fallout that might ensue. |
Retool the setup.py infrastructure
dmcdougall commentedMar 1, 2013
Does this change utilise It might be worth shipping a warning with this. |
mdboom commentedMar 1, 2013
Yes, it does. The fact that we ship copies of so many libraries has long been a problem. There were many good reasons to stop doing that, but also a strong desire to not make the installation from source (or pip) more inconvenient than it already is. The post you linked to is not as alarming as it sounds -- why is he running Maybe we could: a) if the dependencies are missing and distribute is about to download them on our behalf... |
This is a complete reorganization of the
setup.py/setupext.pyinfrastructure, and is the implementation of MEP11.There was some discussion on the mailing list about making dateutil and pyparsing optional dependencies. However, on actually trying to implement that, I'm not sure that makes sense. pyplot imports a number of things directly from dateutil, and we would have to deprecate those things in the API. pyparsing is required merely to parse a font descriptor -- so you'd be losing pretty core functionality without it. I have instead made them hard requirements. The installer will warn if they aren't around, try to install them with pip/easy_install, and failing that importing matplotlib without those dependencies will raise an exception.
[1]http://pyparsing.svn.sourceforge.net/viewvc/pyparsing?view=revision&revision=219