search

leejjoon / pywcsgrid2

astromonical fits image for python matplotlib
http://leejjoon.github.com/pywcsgrid2/
MIT License
25 stars 13 forks source link

Fixed astropy.wcs import #12

Closed astrofrog closed 12 years ago

astrofrog commented 12 years ago

The WCS package in astropy is at astropy.wcs not astropy.pywcs. Also, since astropy.wcs is guaranteed to be a very recent version of pywcs, I thought it would make more sense to try that first. For example, if someone has an old installation of pywcs and astropy, astropy should be preferred (to have the 'sub' attribute).

astrofrog commented 12 years ago

I've just realized I need to add some other commits before this can be merged.

astrofrog commented 12 years ago

Sorry for the premature pull request - I was using the edit function of github and thought I could add edits from multiple files to the same PR but apparently not. I fixed the astropy.wcs imports, but then also changed the PyFITS imports, since if astropy.wcs is used, we need to make sure we are using astropy.io.fits for compatibility. If astropy is not present, pyfits is imported, and pywcs is attempted before kapteyn. So I think it should all work out, but I would recommend testing this with different configurations before merging.

astrofrog commented 12 years ago

I've made a few more fixes. It turns out that if pywcsgrid2 uses astropy internally, one needs to use astropy.io.fits in scripts that use it, rather than PyFITS (at the moment) hence the changes in the examples. However, there are ways to make pywcsgrid2 compatible with both PyFITS and astropy.io.fits input regardless of what pywcsgrid2 uses. I will work on this and will add commits to this PR.

astrofrog commented 12 years ago

I've now added changes to make the code insensitive to whether PyFITS/PyWCS or Astropy are installed, and which one the user uses. Let me know if anything is unclear.

For some reason, if I force pywcsgrid2 to use Kapteyn, I get a lot of errors with the examples, of the form:

AttributeError: 'ProjectionKapteyn' object has no attribute 'substitute'

but that might have been the case before my changes (I will try and check). Also, I noticed some inconsistencies in the code - for example, in wcs_helper.py there is a reference to:

projection = ProjectionKapteynNd(header)

but ProjectionKapteynNd doesn't exist.

I wonder whether it would make sense to migrate to requiring Astropy as the main dependency, and forgetting about PyFITS, PyWCS, and Kapteyn? (it would simplify a lot of code). You'd still need code like what I added that allows the user to pass PyWCS.WCS and PyFITS.Header objects. You could even consider making pywcsgrid2 an affiliated package of Astropy if you like?

Anyway, I understand if you would prefer not to merge this in, or if you need to do some extensive testing, but just wanted to provide it in case it helped.

leejjoon commented 12 years ago

I apologize for my slow response, and I thank you very much for your effort. I will go ahead and merge it.

What I had in my mind is that I make a release as soon as matplotlib 1.2 is out, and then dump all the dependencies on PyFITS, PyWCS, and Kapteyn and move to astropy. So, after I merge your changes, I will make a astropy branch and try to start migration. And any help from you will be greatly appreciated.

And, yes, I think making pywcsgrid2 as an affiliated packages of astropy will be a good idea. I will consider it.