astropy: astropy.io.fits does not handle CONTINUE keywords in FITS headers correctly
Description
The FITS standard allows header keyword values to be longer than 67 characters by using the CONTINUE convention, see this link. The astropy.io.fits module seems to apply some formatting when doing this for better readability, but under some circumstances does not write the correct content to the fits header and omits information.
Expected behavior
If a keyword value is longer than 67 characters, it should get split in 67 character chunks with an ampersand at the end as the 68th character, and then the value should be continued in the next line with the CONTINUE keyword:
In [1]: from astropy.io import fits
...: hdu = fits.PrimaryHDU()
...: keyword = "WHATEVER"
...: value = "SuperCalibrationParameters_XXXX_YYYY_ZZZZZ_KK_01_02_03)-AAABBBCCC.n.h5 SuperNavigationParameters_XXXX_YYYY_ZZZZZ_KK_01_02_03)-AAABBBCCC.n.xml"
...: hdu.header.append((keyword, value, ' '))
...: hdu.header
Out[1]:
SIMPLE = T / conforms to FITS standard
BITPIX = 8 / array data type
NAXIS = 0 / number of array dimensions
EXTEND = T
WHATEVER= 'SuperCalibrationParameters_XXXX_YYYY_ZZZZZ_KK_01_02_03)-AAABBBCCC.n&'
CONTINUE '.h5 SuperNavigationParameters_XXXX_YYYY_ZZZZZ_KK_01_02_03)-AAABBBCC&'
CONTINUE 'C.n.xml'
Actual behavior
The keyword value seems to get reformatted for the FITS header after a blank in the value, and in the last CONTINUE entry, information from the keyword value gets omitted (the “xml” at the end is missing):
In [1]: from astropy.io import fits
...: hdu = fits.PrimaryHDU()
...: keyword = "WHATEVER"
...: value = "SuperCalibrationParameters_XXXX_YYYY_ZZZZZ_KK_01_02_03)-AAABBBCCC.n.h5 SuperNavigationParameters_XXXX_YYYY_ZZZZZ_KK_01_02_03)-AAABBBCCC.n.xml"
...: hdu.header.append((keyword, value, ' '))
...: hdu.header
Out[1]:
SIMPLE = T / conforms to FITS standard
BITPIX = 8 / array data type
NAXIS = 0 / number of array dimensions
EXTEND = T
WHATEVER= 'SuperCalibrationParameters_XXXX_YYYY_ZZZZZ_KK_01_02_03)-AAABBBCCC.n&'
CONTINUE '.h5 &'
CONTINUE 'SuperNavigationParameters_XXXX_YYYY_ZZZZZ_KK_01_02_03)-AAABBBCCC.n.'
Steps to Reproduce
See above.
System Details
Darwin-20.3.0-x86_64-i386-64bit Python 3.7.9 (default, Aug 31 2020, 07:22:35) [Clang 10.0.0 ] Numpy 1.19.2 astropy 4.0.2 Scipy 1.4.1 Matplotlib 3.3.2
About this issue
- Original URL
- State: closed
- Created 3 years ago
- Comments: 15 (10 by maintainers)
Commits related to this issue
- Fix #11298 The original code made the mistake of assuming that the number of groups the string will be split into is directly related to the length of the input or the number of spaces, but this is f... — committed to embray/astropy by embray 3 years ago
- Fix #11298 The original code made the mistake of assuming that the number of groups the string will be split into is directly related to the length of the input or the number of spaces, but this is f... — committed to embray/astropy by embray 3 years ago
- Merge pull request #11304 from embray/issue-11298 Fix #11298 (CONTINUE keyword in header) — committed to astropy/astropy by saimn 3 years ago
- Fix #11298 The original code made the mistake of assuming that the number of groups the string will be split into is directly related to the length of the input or the number of spaces, but this is f... — committed to nstarman/astropy by embray 3 years ago
- Fix #11298 The original code made the mistake of assuming that the number of groups the string will be split into is directly related to the length of the input or the number of spaces, but this is f... — committed to Akshat1Nar/astropy by embray 3 years ago
- Merge pull request #11304 from embray/issue-11298 Fix #11298 (CONTINUE keyword in header) — committed to astropy/astropy by saimn 3 years ago
- Merge pull request #11304 from embray/issue-11298 Fix #11298 (CONTINUE keyword in header) — committed to astropy/astropy by saimn 3 years ago
- Fix #11298 The original code made the mistake of assuming that the number of groups the string will be split into is directly related to the length of the input or the number of spaces, but this is f... — committed to aaryapatil/astropy by embray 3 years ago
Like many things in astropy.io.fits / PyFITS there is history here. This code could easily simply wrap at the character boundary without regard to formatting. Or it could even be replaced with a call to
textwrap.wrap
. The problem is that this specific formatting scheme has been in place for over a decade and if you change it, it will actually cause headers that have been formatted the same way in the past to be formatted differently resulting in unexpected diffs between file versions. It shouldn’t matter of course, but you’d be surprised by how many complaints arise when little things like this have been changed in the past.