-
Notifications
You must be signed in to change notification settings - Fork 82
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
Add Gziped fits file support and AstroImages.jl library #363
Conversation
Codecov Report
@@ Coverage Diff @@
## master #363 +/- ##
==========================================
- Coverage 87.50% 87.16% -0.34%
==========================================
Files 10 10
Lines 696 709 +13
==========================================
+ Hits 609 618 +9
- Misses 87 91 +4
Continue to review full report at Codecov.
|
Three-week bump. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc: @jishnub @mileslucas
@@ -60,6 +59,9 @@ end | |||
|
|||
detect_compressed(io, len=getlength(io); kwargs...) = detect_compressor(io, len; kwargs...) !== nothing | |||
|
|||
const compressed_fits_exten = r"\.(fit|fits|fts|FIT|FITS|FTS)\.(gz|GZ)\>" | |||
name_matches_compressed_fits(io) = (:name ∈ propertynames(io)) && endswith(io.name, compressed_fits_exten) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Curiously, what if propertynames
doesn't contain :name
? Will this break the functionality of detect_rdata
?
How about
name_matches_compressed_fits(io::IOStream) = endswith(io.name, compressed_fits_exten)
name_matches_compressed_fits(io::IO) = false # does FITSIO and AstroImages support generic IO object (e.g., `IOBuffer`)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at the logic of this again, I think it's okay.
If we detect that it's compressed, then if there's no name or the name does not match a compressed fits file, it falls back to the compressed R data path.
Ideally someone with compressed R data could remove the file extension and test this path out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm unexperienced with the Rdata streams, but the astroimages looks good to me- I've tested locally with success with both FITSIO.jl and AstroImages.jl
This PR adds support for loading GZiped FITS files.
Astronomical FITS files are often gziped and given an extension like
.fits.gz
. Compressed fits files can be loaded directly by FITSIO.I did my best to untangle the file loading between plain GZiped files, GZiped RData files, and GZiped fits files.
Everything should work exactly as before unless a filename matches the
.fits.gz
pattern.I believe this also fixes one
#FIXME
comment intests/query.jl
for compressed RData.Finally, I also add support for the AstroImages library as an alternative to FITSIO which was my original purpose 😅.