Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

float decimal format not understood by astropy.io.fits #3422

Merged
merged 3 commits into from Feb 9, 2015

Conversation

embray
Copy link
Member

@embray embray commented Feb 4, 2015

When trying to read data in a fits file I got the following error, while old good pyfits understood data types correctly.

from astropy.io import fits
f = fits.open('file.fits')
f[0].data
$> Format 'F9.0' is not valid--field width and decimal precision must be positive integers

Is there a fix or workaround? Many fits files uses this convention.

@embray embray added the io.fits label Feb 2, 2015
@embray
Copy link
Member

embray commented Feb 2, 2015

Hmm, definitely sounds like a bug. I don't know what the last version of pyfits you used was, but IIRC the code that's parsing these ASCII table format specifiers is relatively new--I need to check back and see if this bug is in pyfits too.

@embray
Copy link
Member

embray commented Feb 3, 2015

Yeah, I'm not sure what I was going off of when I wrote that, but there's no reason the precision field should be non-zero (the standard is a little unclear about this, but I don't see any reason it shouldn't be allowed)

@embray embray added the Bug label Feb 3, 2015
…rash, thinking that the format code was invalid. Also fixes the issue that zero-precision float columns were not allowed, per astropy#3422
…point to the end of the value as long as there is extra space for it. Added an additional test for this case, also covering the bug in astropy#3422.
@embray embray added this to the v1.0.0 milestone Feb 4, 2015
@embray
Copy link
Member

embray commented Feb 4, 2015

Attached a fix for this, hopefully for inclusion in v1.0. Also fixed a minor formatting issue regarding zero-precision fields that I noticed in reading the FITS standard. As far as I can tell it strongly wants us to write a decimal point for float-valued fields, even if there are no digits after the decimal point.

@embray
Copy link
Member

embray commented Feb 4, 2015

Just as a suggestion for a possible workaround (though I haven't tried it) is to change the the column's format string before reading in the data. For example:

>>> hdul = fits.open('path/to/file.fits')
>>> table = hdul[1]
>>> table.header['TFORM1'] = 'F10.1'
>>> table.data

or some variation thereof might work. The bug here only occurs if the "precision" part of the format is zero. But for the purposes of reading in the data it should still work if you change the precision part of the format.

@eteq
Copy link
Member

eteq commented Feb 6, 2015

This looks good to me. And it is a bug fix so fine for v1.0

@toastmaker, can you test this to make sure it works for your file? If we're ready for an RC and you still havn't had a chance to test, we can merge this and there can always be another PR.

@embray
Copy link
Member

embray commented Feb 6, 2015

A sample file would definitely be nice.

@landscape-bot
Copy link

Code Health
Repository health increased by 0.00% when pulling 8662812 on embray:fits/issue-3422 into 6d296b9 on astropy:master.

@eteq
Copy link
Member

eteq commented Feb 9, 2015

I'm going to go ahead and merge this so we can include it in an RC, but a sample file would still be useful, @toastmaker - we can always add it later.

eteq added a commit that referenced this pull request Feb 9, 2015
fix for float decimal format not understood by astropy.io.fits
@eteq eteq merged commit 57ea6a7 into astropy:master Feb 9, 2015
eteq added a commit that referenced this pull request Feb 9, 2015
fix for float decimal format not understood by astropy.io.fits
@embray embray deleted the fits/issue-3422 branch February 9, 2015 18:03
dhomeier pushed a commit to dhomeier/astropy that referenced this pull request Mar 31, 2015
…point to the end of the value as long as there is extra space for it. Added an additional test for this case, also covering the bug in astropy#3422.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants