JuliaAstro / FITSIO.jl

Flexible Image Transport System (FITS) file support for Julia
http://juliaastro.org/FITSIO.jl/
MIT License
55 stars 29 forks source link

Add fitsread and fitswrite #146

Closed jishnub closed 3 years ago

jishnub commented 3 years ago

These convenience functions are useful while working with images. Currently I find this pattern commonly used in my code:

FITS(filename, "r") do f
    read(f[1])
end

this may be replaced by

fitsread(filename)

and similarly for writing as well. This is easier for REPL usage too.

Not sure if these should be exported (possible conflicts in case users have defined these themselves), so currently it's up to the users to import these.

mileslucas commented 3 years ago

Thanks for this! I agree, we've been needing a quick one-liner for a while now. Some considerations:

pyfits/astropy.io.fits both have some naming conventions that we can choose to follow. For reading, they use getdata (and a corresonding getheader) and for writing they use writeto and append. It would be easier for users to transition between libraries if we adopt these, although I don't find the verbage as consistent or accurate as fitsread and fitswrite.

Edit: you already had the optional header argument!

giordano commented 3 years ago

Good point about having a name that this easier to guess for users coming from Astropy.

It would be good to have a docstring. Also, export and add these functions to docs/src/api.md if the idea is to make them "public".

mileslucas commented 3 years ago

For these functions, too, they should include clear documentation about the awkward dimension permutation that happens in the c/Julia interop. Someone looking for the "how do I read data" as soon as possible probably will trip over the nuance of the column-major arrays.

jishnub commented 3 years ago

Yes I was planning to add a docstring, didn't have time for this earlier. About the name - I chose this as matlab uses these names, although they don't match the astropy ones. Personally I find getdata to be a rather uninformative name if the context isn't clear, although I can use this if there's a consensus.

ajwheeler commented 3 years ago

Should there be an optional parameter to change which HDU is read for multi-extension files? I most often need f[2].

giordano commented 3 years ago

Should there be an optional parameter to change which HDU is read for multi-extension files?

It's there already, hduindex

ajwheeler commented 3 years ago

🤦 , sorry.