search

krakrjak / fits-parse

Parse FITS files for Astronomy data analysis.
BSD 2-Clause "Simplified" License
2 stars 1 forks source link

Support for Extensions, Complete Headers and Multiple HDUs #2

Closed seanhess closed 11 months ago

seanhess commented 11 months ago

As described in discussion in #1, adds more robust support for many things in order to parse FITS files from the NSO L1 Data Products.

Since we last spoke I added several things, including full support for BINTABLE, a method to read specific headers, and multiple refactors to better support extensions. Sorry for such a huge PR. Turns out our FITS files used just about every features in the spec!

There are still a few cleanup things to do, comments, formatting, etc, but I thought it would be good to get the PR in.

seanhess commented 11 months ago

I'm the most unsure about the Data.Fits.Read module, which was a last minute addition. It makes it easy to parse the file and read specific headers in the Either String monad.

The Either String approach has the advantage of being intuitive and easily combinable with other validators. One alternative is to expose our own Parser monad (a-la-Aeson), and/or support parsing into Generic records. With a custom monad we could carry the HDU as context instead of passing it to the parsing functions, making it ever so slighly cleaner. The main disadvantage of these approaches is end-user complexity, possible confusion with the Megaparsec Parser, and preventing them from using Either String to perform other validations at the same time.

I think maybe the correct design won't emerge until we start writing real code against the library, which I will be doing in within the next month.

seanhess commented 11 months ago

@krakrjak Have you had a chance to look at this?

seanhess commented 11 months ago

Pushed a change of all Natural -> Int.

Any thoughts on the larger refactors?