esheldon / fitsio

A python package for FITS input/output wrapping cfitsio
GNU General Public License v2.0
133 stars 57 forks source link

HIERARCH + CONTINUE doesn't work. #358

Closed rmjarvis closed 4 months ago

rmjarvis commented 1 year ago

fitsio can read/write long value items into a header using CONTINUE lines. And it can read/write long keyword names with HIERARCH lines. However, combining the two fails. Here is a quick unit test, which fails:

import fitsio
import numpy as np

header={
    "short1": "Short string",
    "short2": "A value string that is very long.  It needs to be at least 80 characters to trigger using CONTINUE, which this is.",
    "long_keyword_name1": "Another short string",
    "long_keyword_name2": "Another very long string that again must be at least 80 characters.  Probably less this time since it also uses HIERARCH."
}

with fitsio.FITS('temp.fits', 'rw', clobber=True) as f:
    f.write(np.array([1,2,3]), header=header)

with fitsio.FITS('temp.fits') as f:
    header2 = f[0].read_header()

for key in header:
    print(f"{key}\n    {header[key]}\n    {header2[key]}\n")
    assert header[key] == header2[key]

Output:

short1
    Short string
    Short string

short2
    A value string that is very long.  It needs to be at least 80 characters to trigger using CONTINUE, which this is.
    A value string that is very long.  It needs to be at least 80 characters to trigger using CONTINUE, which this is.

long_keyword_name1
    Another short string
    Another short string

long_keyword_name2
    Another very long string that again must be at least 80 characters.  Probably less this time since it also uses HIERARCH.
    Another very long string that again must be at le

Traceback (most recent call last):
  File "/Users/Mike/GalSim/tmp.py", line 19, in <module>
    assert header[key] == header2[key]
AssertionError

Note: This came up on LSST Slack (see thread on software-dev channel Jan 24, 10:28 AM EST). It seems to be a bug in the backend cfitsio implementation. Tim Jenness sent an email to cfitsio devs to report the issue. I'm not sure if it's possible for fitsio to work around this or not to fix the bug sooner, rather than just wait for cfitsio to fix it.

timj commented 1 year ago

In my quick test with Rubin code using cfitsio I ended up with a header that looks like this:

NOSPACE = 'test1   '
HIERARCH WITH SPACE = 'test2   '
LONGKWD = '0123456789012345678901234567890123456789012345678901234567890123456&'
CONTINUE  '7890    '
HIERARCH LONGKWD SPACE= '012345678901234567890123456789012345678901234567890123'
CONTINUE  '1234567890'

so the & is missing from the end and 7 characters have been dropped. I sent an email to the cfitsio help desk.

beckermr commented 1 year ago

Fitsio bundles cfitsio due to some features around Unicode strings that have not been upstreamed. If you have a patch that fixes the bug, we can add it to the patches we carry locally.

esheldon commented 4 months ago

Any news on this one?

timj commented 4 months ago

I haven't tried v4.4.0 of cfitsio but the release notes on https://heasarc.gsfc.nasa.gov/FTP/software/fitsio/c/docs/changes.txt mention long keywords but not HIERARCH so I don't know if it got fixed. I definitely didn't get an email about it from them and I sent a follow up to my email from 18 months ago.

timj commented 4 months ago

I am told this was fixed in cfitsio 4.3.0:

Enhancement of long string keyword read/write functions to fully conform with FITS standard specifications for multi-line value and comment strings. Two new functions have been added to implement this: fits_get_key_com_strlen and fits_read_string_key_com.

esheldon commented 4 months ago

I'll look into bundling 4.4.0 and using the new functions

esheldon commented 4 months ago

Moving to 4.4.0 fixed the bug without any changes in this package