ejeschke / ginga

The Ginga astronomical FITS file viewer
BSD 3-Clause "New" or "Revised" License
122 stars 77 forks source link

use astropy.wcs for figuring out WCS #18

Closed eteq closed 11 years ago

eteq commented 11 years ago

Right now it looks like the WCS solutions come from ginga's own wcs.py. I've run into a number of FITS images that don't work quite right I think just because they have one of the bajillion non-standard WCS forms. I think switching to something like astropy.wcs would make a lot of these work, as it's based on a C library with a long history of supporting poorly-written FITS files...

It also means anyone wanting to write a plugin in another affiliated package only needs to undestand one sort of WCS object.

ejeschke commented 11 years ago

Ginga uses astropy.wcs if it is installed. If it is not installed it looks for pywcs. If that is not installed it falls back to the bare bones wcs. So something else is likely amiss. Can you do this:

from ginga.wcs import have_astropy have_astropy True

If this works then Ginga should be using astropy's wcs.

Do you have a sample of the fits file with the strange wcs?

Coordinate system choice can be made via Preferences plugin under the WCS header.

eteq commented 11 years ago

Ah, I didn't realize that it did that - well, the actual title is a bad one then - sorry about that!

The specific error that prompted me to issue this is still happening, though, so I'm thinking either it's not switching over to astropy.wcs or this problem is actually before it starts parsing the wcs.

I'm seeing the same error with at least two different data sources very recently - one is a raw data file from the WIYN HYDRA spectrograph, and the other is from KECK/ESI. I've included the traceback ginga generates below.

Failed to load file 'ccd189.fits': 'CTYPE1'
  File "/Users/erik/src/ginga/build/lib/ginga/Control.py", line 333, in load_image
    image.load_file(filepath)

  File "/Users/erik/src/ginga/build/lib/ginga/AstroImage.py", line 103, in load_file
    self.load_hdu(hdu, fobj=fits_f, naxispath=naxispath)

  File "/Users/erik/src/ginga/build/lib/ginga/AstroImage.py", line 62, in load_hdu
    self.update_keywords(hdu.header)

  File "/Users/erik/src/ginga/build/lib/ginga/AstroImage.py", line 180, in update_keywords
    self.wcs.load_header(hdr)

  File "/Users/erik/src/ginga/build/lib/ginga/wcs.py", line 106, in load_header
    self.coordsys = choose_coord_system(self.header)

  File "/Users/erik/src/ginga/build/lib/ginga/wcs.py", line 420, in choose_coord_system
    ctype = header['CTYPE1'].strip().upper()

I think the underlying problem is that they don't have a 'CTYPE1' - this makes sense for raw data where nothing has been calibrated and the only meaningful coordinates are the raw image coordinates.

ejeschke commented 11 years ago

Excellent, now we are getting somewhere. Ginga makes a rough guess at the initial astropy.wcs.coordinates class to use based on the FITS header. For the particular file that you are loading, should that default to ICRSCoordinate? I need to modify the choose_coord_system function. BTW, it seems like this would be a very useful function for astropy to have built in somewhere--see the comments at the bottom of issue 15 (https://github.com/ejeschke/ginga/issues/15)

eteq commented 11 years ago

Hmm, well in this case it is not a celestial coordinate system - it should instead just be raw pixel/image coordinates (or linear world coordinates with deltas of 1 and 0-points of 0). So it doesn't really map onto a celestial coordinate. There should be a WCS representation of this (CTYPE1/2 are linear, CRVAL1/2 are 0, and CDELT1/2 are 1, I suppose), but it's not actually in the fits itself, so I suppose it would need to be mocked up.

ds9 handles this by having a separate "image coordinates" it uses if there's no WCS...

ejeschke commented 11 years ago

@eteq, could you check out the latest commit with your image? It should now load fine and the message as you mouse around should be 'NO WCS'. Pixel and value info is listed.

eteq commented 11 years ago

Yep, that did the trick - thanks! I'll go ahead and close this issue then, as the original cause for it has been addressed.