Skip to content

Issue with unit 'HZ' in FITS files (astropy.wcs) - #36

Merged
mdboom merged 3 commits into
astropy:masterfrom
mdboom:wcs/fix_kwarg
Nov 1, 2011
Merged

Issue with unit 'HZ' in FITS files (astropy.wcs)#36
mdboom merged 3 commits into
astropy:masterfrom
mdboom:wcs/fix_kwarg

Conversation

@mdboom

@mdboom mdboom commented Oct 28, 2011

Copy link
Copy Markdown
Contributor

It seems that astropy.wcs (and more specifically wcslib) has issues with the unit 'HZ'. For example, the following header:

SIMPLE  =                    T /Standard FITS
BITPIX  =                  -32 /Floating point (32 bit)
NAXIS   =                    4
NAXIS1  =                  500
NAXIS2  =                  500
NAXIS3  =                  120
NAXIS4  =                    1
EQUINOX =   2.000000000000E+03
PC001001=   1.000000000000E+00
PC002001=   0.000000000000E+00
PC003001=   0.000000000000E+00
PC004001=   0.000000000000E+00
PC001002=   0.000000000000E+00
PC002002=   1.000000000000E+00
PC003002=   0.000000000000E+00
PC004002=   0.000000000000E+00
PC001003=   0.000000000000E+00
PC002003=   0.000000000000E+00
PC003003=   1.000000000000E+00
PC004003=   0.000000000000E+00
PC001004=   0.000000000000E+00
PC002004=   0.000000000000E+00
PC003004=   0.000000000000E+00
PC004004=   1.000000000000E+00
CTYPE1  = 'RA---SIN'
CRVAL1  =   1.000000000000E+01
CDELT1  =  -2.000000000000E-05
CRPIX1  =   2.510000000000E+02
CUNIT1  = 'deg     '
CTYPE2  = 'DEC--SIN'
CRVAL2  =   1.000000000000E+01
CDELT2  =   2.0000000000000-05
CRPIX2  =   2.510000000000E+02
CUNIT2  = 'deg     '
CTYPE3  = 'FREQ    '
CRVAL3  =   2.000000000000E+11
CDELT3  =   1.000000000000E+05
CRPIX3  =   1.000000000000E+00
CUNIT3  = 'HZ      '
CTYPE4  = 'STOKES  '
CRVAL4  =   1.000000000000E+00
CDELT4  =   1.000000000000E+00
CRPIX4  =   1.000000000000E+00
CUNIT4  = '        '
PV2_1   =   0.000000000000E+00
PV2_2   =   0.000000000000E+00

parsed with:

from astropy.wcs import WCS
import pyfits

header = pyfits.Header()
header.fromTxtFile('header.hdr')
wcs = WCS(header)
wcs.wcs_sky2pix([[12.,13.,0.3, 0.4]], 0)

gives the error:

Traceback (most recent call last):
  File "test.py", line 7, in <module>
    wcs.wcs_sky2pix([[12.,13.,0.3, 0.4]], 0)
  File "/Users/tom/Library/Python/2.7/lib/python/site-packages/astropy-0.0dev_r220-py2.7-macosx-10.6-x86_64.egg/astropy/wcs/wcs.py", line 783, in wcs_sky2pix
    'input', *args, **kwargs)
  File "/Users/tom/Library/Python/2.7/lib/python/site-packages/astropy-0.0dev_r220-py2.7-macosx-10.6-x86_64.egg/astropy/wcs/wcs.py", line 613, in _array_converter
    result = func(xy, origin)
  File "/Users/tom/Library/Python/2.7/lib/python/site-packages/astropy-0.0dev_r220-py2.7-macosx-10.6-x86_64.egg/astropy/wcs/wcs.py", line 782, in <lambda>
    lambda xy, o: self.wcs.s2p(xy, o)['pixcrd'],
astropy.wcs._wcs.InvalidTransformError: ERROR 6 in wcs_units() at line 2009 of file /Users/tom/tmp/astropy_install/astropy/wcs/src/wcslib/C/wcs.c:
  In CUNIT2 : Invalid symbol in EXPON context in 'HZ'.

Does the WCS standard really forbid the uppercase HZ? If so, could you point me to where this is defined, so I can report a bug with the software that produced this file? And do we want to relax astropy.wcs so that it catches that since I've found a number of 'real' datasets that use `HZ``?

@ghost ghost assigned mdboom Oct 28, 2011
@mdboom

mdboom commented Oct 28, 2011

Copy link
Copy Markdown
Contributor

Units in FITS are case-sensitive by definition. Look at "s" (seconds) vs.
"S" (siemens).

The relevant FITS standard can be downloaded here:
http://www.aanda.org/index.php?option=com_article&access=doi&doi=10.1051/0004-6361/201015362&Itemid=129
.
See Tables 3 and 4 on pages 9 and 10. I would report this as a bug to the
software that produced the file.

I don't think it's a great idea to relax the standard within astropy.wcs --
there is too much potential for breakage and ambiguity. However, wcslib
(and therefore astropy.wcs) offers a function to automatically fix commonly
found deviations from the standard. Call:

wcs.wcs.unitfix()

This was not very well documented in the latest release of pywcs, but
astropy and pywcs repositories have good documentation about how units work
here:

https://github.com/astropy/astropy/blob/master/docs/wcs/api_units.rst

(I hope to get a new release of pywcs out soon, so this improved
documentation will go online.)

Mike

Michael Droettboom
http://www.droettboom.com/

@mdboom

mdboom commented Oct 28, 2011

Copy link
Copy Markdown
Contributor

An alternative (though a diversion from wcslib behavior), may to add a kwarg to the WCS() constructor "fix=True", then when True would call all of the fixers. The user could set this to False if they know they don't want any fixing. I would also like to emit a warning when any fixers are performed so that deviations-from-the-standard are known and reported back to the producing software.

@mdboom
Add a kwarg 'fix' to WCS constructor. When 'True' (default), fix() is…
37c2ee5 
… called on the wcsprm object upon construction. Warnings are emitted if any non-standard content was found and changes were made.
@astrofrog

astrofrog commented Oct 29, 2011

Copy link
Copy Markdown
Member Author

Thanks! Using wcs.wcs.unitfix() works great, but I like the addition of the fix=True option. Feel free to merge.

mdboom added 2 commits October 31, 2011 08:33
@mdboom
Add test data.
8505dfc
@mdboom
Add a patch against wcslib for displaying the unit translation in the…
04f7c39 
… error message. This needs to be maintained until it is accepted upstream.
@eteq

eteq commented Oct 31, 2011

Copy link
Copy Markdown
Member

Looks fine for merging to me as well - @mdboom, I see you recently made a change, though... I'll leave it to you to merge if you're done with changes.

mdboom added a commit that referenced this pull request Nov 1, 2011
@mdboom
Merge pull request #36 from mdboom/wcs/fix_kwarg
b6d1e3b 
Issue with unit 'HZ' in FITS files (astropy.wcs)
@mdboom
mdboom merged commit b6d1e3b into astropy:master Nov 1, 2011
keflavich pushed a commit to keflavich/astropy that referenced this pull request Oct 9, 2013
@mdboom
Merge pull request astropy#36 from mdboom/wcs/fix_kwarg
379af79 
Issue with unit 'HZ' in FITS files (astropy.wcs)
@mdboom
mdboom deleted the wcs/fix_kwarg branch May 22, 2014 00:00
eteq added a commit that referenced this pull request Feb 9, 2015
@eteq
Merge pull request #36 from cdeil/address-3359-changes
f5af2b4 
Docs fixes in matchsep.rst
@mdboom mdboom mentioned this pull request Apr 17, 2015
Closed
@nden nden mentioned this pull request Feb 3, 2016
Merged
@meresmclr meresmclr mentioned this pull request Feb 29, 2016
Merged
jeffjennings pushed a commit to jeffjennings/astropy that referenced this pull request Jul 2, 2025
@sudheesh001
Fixes Issue astropy#36 fixes broken link and missing image from old_s…
7a37a55 
…ite branch to master
jeffjennings pushed a commit to jeffjennings/astropy that referenced this pull request Jul 2, 2025
@astrofrog
Merge pull request astropy#43 from sudheesh001/Patch_36
c1422e8 
Fixes Issue astropy#36 fixes broken link and missing image from old_site branch to master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@mdboom mdboom

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@mdboom @astrofrog @eteq