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

v0.16.7 release breaks string keys? #163

Closed andrew-saydjari closed 2 years ago

andrew-saydjari commented 3 years ago

After a recent upgrade to v0.16.7 from v0.16.3 I noticed that querying HDUs by name (rather than integer index) seems to be no longer possible? Was this intentional? If it was, it might be nice to keep for back compatibility reasons.

mileslucas commented 2 years ago

Hi, I'm going to look into this- can you provide a quick code snippet of the behavior you were expecting? Thanks!

andrew-saydjari commented 2 years ago

Previously I could do the following.

f = FITS("test.fits","w")
write(f,ones(10,10),name="a")
read(f["a"])
read(f[1])

and both of the last two lines worked. After the update to the newest version, only the last line seemed to work for me.

giordano commented 2 years ago

I'm a bit confused. Your example works for me with this patch:

diff --git a/src/fits.jl b/src/fits.jl
index 3ee72b6..15b669a 100644
--- a/src/fits.jl
+++ b/src/fits.jl
@@ -111,7 +111,7 @@ end
 function getindex(f::FITS, name::AbstractString, ver::Int=0)
     fits_assert_open(f.fitsfile)
     fits_movnam_hdu(f.fitsfile, name, ver)
-    i = fits_get_hdu_num(f.fitsfile)
+    i = Int(fits_get_hdu_num(f.fitsfile))

     if haskey(f.hdus, i)
         return f.hdus[i]

but as far as I could see the relevant lines (definition of ImageHDU and getindex(f::FITS, name::AbstractString, ver::Int=0)) haven't been changed in the last 6 years. Edit: nevermind, I think this is due to #158 which changed the inner constructor of ImageHDU

andrew-saydjari commented 2 years ago

I just get the following error which I guess wrapping with Int takes care of by converting the Int32 to Int64. So, I guess this is a simple fix.

`MethodError: no method matching ImageHDU(::CFITSIO.FITSFile, ::Int32) Closest candidates are: ImageHDU(::CFITSIO.FITSFile, ::Int64) at /n/home12/saydjari/.julia/packages/FITSIO/dBcm0/src/FITSIO.jl:106

Stacktrace: [1] getindex(::FITS, ::String, ::Int64) at /n/home12/saydjari/.julia/packages/FITSIO/dBcm0/src/fits.jl:121 [2] getindex(::FITS, ::String) at /n/home12/saydjari/.julia/packages/FITSIO/dBcm0/src/fits.jl:112

[4] include_string(::Function, ::Module, ::String, ::String) at ./loading.jl:1091`