search

astrogo / fitsio

fitsio is a pure-Go package to read and write `FITS` files
BSD 3-Clause "New" or "Revised" License
53 stars 24 forks source link

Handling of COMMENT, HISTORY, END cards differs wrt cfitsio #34

Open airnandez opened 6 years ago

airnandez commented 6 years ago

The returned value of Header.Keys() does not include cards of type END, COMMENT, HISTORY:

https://github.com/astrogo/fitsio/blob/0221d1a54f8c95dcfc14c3234c9091e3287ef411/header.go#L323

I understand that there is a method for retrieving the comments and history cards of a FITS header, I would suggest that for compatibility with cfitsio, those cards should be also be returned by Header.Keys().

In addition, it seems that astrogo/cfitsio assumes that in a given HDU there is only one card of type HISTORY or COMMENT. This is not the case with cfitsio.

In case it matters, for my tests I'm using cfitsio v3.370.

An example of a file with several COMMENT cards in the same HDU can be found at:

ftp://legacy.gsfc.nasa.gov/fits_info/sample_files/images/rosat_pspc_rdf2_3_bk1.fits

sbinet commented 6 years ago

this begs the question as whether we shouldn't just return the []Card, or have a method to retrieve the n-th card (otherwise, we'd always get the first occurence of COMMENT or HISTORY via the Header.Get(name string) method... or rename the currrent Get(string) into GetByName(string) and introduce Get(int). but Header.Cards() []Card SGTM (carefully documenting one shouldn't mess with modifying their content.)

airnandez commented 6 years ago

For my use case, which basically consists of scanning all the cards in all header units of a given file and perform some actions depending on the value of some of them, Header.Cards() []Card would be perfect and much more efficient indeed that to repeatedly call Header.Get(key). A note in the documentation could remind the users that they should consider the returned Card slice data as read-only.

In that case, Header.Cards() []Card should return all the cards (including SIMPLE, COMMENT, HISTORY, END) and preserve their order as found in the file.

I would suggest to keep Header.Get(key) which is convenient and modify Header.Comments() []string (note the plural) to return several values, instead of just one value as they currently does. I don't know for sure if a well formed FITS file is supposed to have several HISTORY cards: if that's the case, Header.History() could also become Header.Comments() []string.

sbinet commented 6 years ago

apologies for the belated answer.

do you want to give this a go and send a PR?