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

relax eltypes in inplace read #166

Closed jishnub closed 2 years ago

jishnub commented 2 years ago

The PR is best viewed by ignoring the whitespace fixes. The main change is in the various read! functions that now accept an array with an eltype different from that of the image. This is supported by cfitsio through an internal conversion.

After this PR

julia> f = tempname()
"/tmp/jl_ILWosZ"

julia> FITSIO.fitswrite(f, [1 2; 3 4]);

julia> m = zeros(2,2);

julia> ff = FITS(f, "r");

julia> read!(ff[1], m)
2×2 Matrix{Float64}:
 1.0  2.0
 3.0  4.0

julia> m
2×2 Matrix{Float64}:
 1.0  2.0
 3.0  4.0

julia> eltype(ff[1])
Int64

Here Int64 data is read into a Float64 array. This used to error on master.

codecov[bot] commented 2 years ago

Codecov Report

Merging #166 (f6b92cf) into master (3ec2708) will increase coverage by 0.14%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #166      +/-   ##
==========================================
+ Coverage   91.20%   91.35%   +0.14%     
==========================================
  Files           5        5              
  Lines         614      613       -1     
==========================================
  Hits          560      560              
+ Misses         54       53       -1     
Impacted Files Coverage Δ
src/image.jl 95.58% <100.00%> (+0.69%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 3ec2708...f6b92cf. Read the comment docs.

giordano commented 2 years ago

More conflicts also here

jishnub commented 2 years ago

Fixed now, but there may be more if other PRs are merged :smiley:

giordano commented 2 years ago

Yeah, conflicts again