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

Iterate over the cards of an HDU header. #374

Open
chrsoo opened this issue Dec 29, 2024 · 1 comment
Open

Iterate over the cards of an HDU header. #374

chrsoo opened this issue Dec 29, 2024 · 1 comment

Comments

@chrsoo
Copy link
Contributor

chrsoo commented Dec 29, 2024

Hi,

I would like to iterate over all the cards in the header of an HDU for use in my own tool developed in Rust. From what I can tell the current rust-fitsio API assumes prior knowledge of all header keyword names, something I would like to avoid.

I did not find this in the list of features documented in issue #15.

Iterating over the cards is possible by using fits_get_hdrspace for finding out the total number of cards and then retrieve them by number through fits_read_record or fits_read_keyn, as indicated in section 5.4.1 of the cfitsio documentation.

I have taken a first stab at this, but before creating a PR that respects the contributing guidelines, it would be great if you could provide some initial feedback, specfically:

  • Does support for iterating over the header cards in an HDU seem like a reasonable addition to the API?
  • In my first attempt I introduced a Card struct to conveniently hold the card data. Given that cards in the Rust fitsio API are implements as key/value and key / (value, comment) it may be preferable to use tuples, what would you suggest?
  • Assuming Cards are a "go", a better idea could be to read the entire card record into one array (using ffgrec) and use slices to reference the right section in the buffer. (Being a Rust neophyte, I thought the approach of using individual buffers (and ffgkyn) was easier...)

Best regards,

Chris

@simonrw
Copy link
Owner

simonrw commented Dec 29, 2024

Hi @chrsoo , thanks for making this suggestion.

I would like to iterate over all the cards in the header of an HDU for use in my own tool developed in Rust. From what I can tell the current rust-fitsio API assumes prior knowledge of all header keyword names, something I would like to avoid.

That is a great suggestion, thank you!

I did not find this in the list of features documented in issue #15.

That issue has not been updated regularly, so is likely out of date, however looking through the documentation I agree that this feature is not supported yet.

Iterating over the cards is possible by using fits_get_hdrspace for finding out the total number of cards and then retrieve them by number through fits_read_record or fits_read_keyn, as indicated in section 5.4.1 of the cfitsio documentation.

That's a good idea, thank you.

I have taken a first stab at this, but before creating a PR that respects the contributing guidelines, it would be great if you could provide some initial feedback, specfically:

Does support for iterating over the header cards in an HDU seem like a reasonable addition to the API?
In my first attempt I introduced a Card struct to conveniently hold the card data. Given that cards in the Rust fitsio API are implements as key/value and key / (value, comment) it may be preferable to use tuples, what would you suggest?

I think a struct is definitely preferable in general.

Assuming Cards are a "go", a better idea could be to read the entire card record into one array (using ffgrec) and use slices to reference the right section in the buffer. (Being a Rust neophyte, I thought the approach of using individual buffers (and ffgkyn) was easier...)

I think there are two considerations:

  1. how do we read/buffer all of the records, and
  2. how do we read/buffer each individual record.

I can see a potential for using something like fits_hdr2str to optimise 1., however that is a future concern.

Regarding the suggestions for 2.: I would have a preference for using fits_read_keyn / ffgkyn initially to ensure the correct behaviour, as using fits_read_record / ffgrec is a performance optimisation we don't need right now.

I am happy to continue further discussions in the PR, but in general I am on board with this idea, thank you!

simonrw pushed a commit to chrsoo/rust-fitsio that referenced this issue Jan 2, 2025
simonrw pushed a commit to chrsoo/rust-fitsio that referenced this issue Jan 2, 2025
simonrw pushed a commit to chrsoo/rust-fitsio that referenced this issue Jan 2, 2025
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