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

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

Open
airnandez opened this issue Dec 5, 2017 · 3 comments
Open

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

airnandez opened this issue Dec 5, 2017 · 3 comments
Assignees

Comments

@airnandez
Copy link
Contributor

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

func (hdr *Header) Keys() []string {

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 sbinet self-assigned this Dec 5, 2017
@sbinet
Copy link
Contributor

sbinet commented Dec 5, 2017

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
Copy link
Contributor Author

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
Copy link
Contributor

sbinet commented Dec 12, 2017

apologies for the belated answer.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants