ericmandel / pyds9

Python connection to SAOimage DS9 via XPA
76 stars 41 forks source link

Provide a way to use installed xpa library #26

Open olebole opened 8 years ago

olebole commented 8 years ago

For Debian, I need to use the xpa package provided by Debian instead of the convenience copy that comes with pyds9. Other distributions (Fedora) may have the same requirement.

Therefore it would be nice to have an option to use an already installed XPA library (header and development files).

montefra commented 8 years ago

@olebole : I am restructuring the directory structure using astropy helpers.

Once I'll get the installation working again I'll see how to do it. @cdeil mentioned that other astropy affiliated packages have the possibility to use system libraries for some c-extension, so should be possible.

I'll update you when I'll have more info.

cdeil commented 8 years ago

@montefra – I don't remember if this has been discussed or is technically possible / easy to implement, but if it is, the best (easiest to maintain / package) solution would be to separate pyds9 from xpa completely. Supporting both options (bundled or system) means extra effort to get the build scripts right, set up testing for more configs, deal with more diverse install issues from users ... is it worth the effort?

I might be wrong, but I think none of the Astropy-affiliated packages (see http://www.astropy.org/affiliated/) bundles things in cextern, only Astropy core bundles a handful: https://github.com/astropy/astropy/tree/master/cextern So if you're looking for an example package that has implemented options to optionally use system C libraries, look in the Astropy core package.

ericmandel commented 8 years ago

@cdeil separating xpa is fine if it does not require two steps to install pyds9 (i.e. installing xpa first).

@montefra One problem with picking up an installed version will be knowing the installed location, since it's not mandatory to install xpa in a directory automatically searched by gcc/ld. Aside from that, in setup.py/make, could we simply search the system directories for libxpa.* and xpa.h and, if found, modify the data_files variable and skip the build? Or perhaps Python has an easy way to mimic gnu configure's AC_CHECK_LIB macro: https://www.gnu.org/savannah-checkouts/gnu/autoconf/manual/autoconf-2.69/html_node/Libraries.html?

cdeil commented 8 years ago

@cdeil separating xpa is fine if it does not require two steps to install pyds9 (i.e. installing xpa first).

Ah, I think now I understand ... XPA is a C library only, so pip install pyds9 (what many Python users use) can't require XPA as a dependency (because there's no package for XPA on PyPI)

So the options are:

I guess changing the location of the Python wrapper (from pyds9 package to the xpa or pyxpa package mostly makes sense if it's re-usable by other packages.

We've discussed this, but has there been a decision on this?

If this is still unclear, maybe keeping XPA and the Python wrapper bundled in the pyds9 package distributed on PyPI, and adding an option to use the system XPA version, is the pragmatic solution?

@montefra @ericmandel What do you think?

One problem with picking up an installed version will be knowing the installed location

I usually call pkg-config via subprocess from setup.py to find out that info. In my experience it works very well for Linux and Mac, but I don't know about Windows. Is this an option, to just use pkg-config and to require that it's set up correctly for users that want to use system libraries? If yes, it's pretty straight-forward to implement, I think. If no, the best guess is probably to look into astropy-helpers or directly ask for help / advice from the astropy-helpers author, Erik Bray.

olebole commented 8 years ago

I don't need an automatic solution here; something like --use-system-libraries (like for astropy) would be sufficient to use the library installed in standard paths. When used this flag, the automated build of xpa should be switched off, and in xpa.py, the _libpath should be set to libxpa.so.1. Similarly, the xpans binary should just be looked up in the standard PATH in pyds9.py. See the patch for the Debian package for a template.

cdeil commented 8 years ago

As one extra data point, I'm on Mac and am using Macports.

Based on the xpa portfile I currently get:

$ port contents xpa
Port xpa contains:
  /opt/local/bin/xpaaccess
  /opt/local/bin/xpaget
  /opt/local/bin/xpainfo
  /opt/local/bin/xpamb
  /opt/local/bin/xpans
  /opt/local/bin/xpaset
  /opt/local/include/prsetup.h
  /opt/local/include/xpa.h
  /opt/local/lib/libxpa.a

and based on the pyds9 portfile I have:

$ port contents py27-pyds9
Port py27-pyds9 contains:
  /opt/local/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/site-packages/ds9.py
  /opt/local/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/site-packages/ds9.pyc
  /opt/local/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/site-packages/libxpa.dylib
  /opt/local/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/site-packages/pyds9-1.1-py2.7.egg-info
  /opt/local/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/site-packages/xpa.py
  /opt/local/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/site-packages/xpa.pyc
  /opt/local/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/site-packages/xpans

If the recommendation "in xpa.py, the _libpath should be set to libxpa.so.1" is followed, this would mean it has to be patched by Mac users, whereas at the moment it just works (no patching needed). So I think hardcoding libxpa.so.1 isn't a good solution.

olebole commented 8 years ago

The macport xpa package does not provide a dynamic library, so --use-system-libs would be not usable for Mac users anyway :-) However, you are right: one should use the according methods from astropy-helpers here.

ericmandel commented 8 years ago

Add a Python wrapper to XPA Put XPA and the Python wrapper to a separate pyxpa package on PyPI Keep XPA and the Python wrapper in the pyds9 repo.

Outside of its use in DS9, I don't think there is much call for using XPA in general, or an XPA connection to Python in particular. XPA is pretty old technology (from the 1990s) so I doubt any new projects would (or should) use it. So I don't think separating XPA from pyds9 would be worth the effort.

I usually call pkg-config via subprocess from setup.py to find out that info. In my experience it works very well for Linux and Mac, but I don't know about Windows.

Right, thanks! I forgot about pkg-config. XPA should have a xpa.pc file but I see it does not. So I will add one and we can take it from there. Since the pyds9 build of XPA already checks the current host for the library extension (to differentiate Mac from Linux from Windows), we probably can add another check pretty easily to bypass the build entirely. This would be automatic once the xpa pkg-config is in place, while preserving the library name properly.

ericmandel commented 8 years ago

@olebole @montefra @cdeil I added pkg_config support to the main XPA repository (and also got rid of a few new clang warnings in tcl.c, which is not compiled by pyds9). The new XPA version is 2.1.17. We should now be able to run pkg-config inside setup.py to determine whether a given host has the XPA libraries already installed. (I can't do this right now, but can work on it later in the month, unless @montefra gets to it first).

montefra commented 8 years ago

@ericmandel: thank for it. I'll update the xpa submodule tomorrow. If I can figure out how astropy build stuff in cextern I'll try to do it. Anyway on Sunday I'll leave for a 2 weeks trip and I don't know if/how much time I'll have.

ericmandel commented 8 years ago

We certainly can leave this until you get back from your trip!

montefra commented 8 years ago

@olebole: I've began to work the build and installation of xpa using the astropy_helpers machinery (#28). This should provide the solution for your request without too much trouble (I hope).

I hope that it will not take too much time to finish this.