Repository navigation
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #32 +/- ##
==========================================
+ Coverage 94.11% 96.96% +2.85%
==========================================
Files 4 3 -1
Lines 85 66 -19
==========================================
- Hits 80 64 -16
+ Misses 5 2 -3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
barrettp
left a comment
There was a problem hiding this comment.
I think that you are getting ahead of yourself with these changes by assuming that the FITSFiles.jl PR #45 will be approved. The proposed changes to PR #45 need to be investigated to ensure that they are consistent with the design philosophy of FITSFiles.jl before it is approved.
As for this PR (#32), I suggest that the hdu_in argument have a type qualifier. It is not clear by looking at the code that it is an integer, string, or both. In addition, what is the point of having this function when fits.data[n] performs the same function? Is this a vestige of using FITSIO.jl?
Not trying to assume, apologies if that is what I made it seem like. I'm just trying to experiment a bit to see what shakes out, which is why this is currently a draft PR and why I have been making such heavy use of I've just responded to your comments in #45 to try and express why I think this experiment is worth investigating
Two shakes, will take a look and get back to you Update: Thanks for pointing this. I've tightened the signatures, which then let me extend our HDU selection scheme so that folks can select by int, string, or symbol now (cfc9fcd)
Yea, it totally is, haha. Here's a pass at simplifying things further (e9c43e9). I've included a summary about this under the "Simplified input handling" section in the updated PR description 👍🏾 |
Migrates Reproject.jl from the cfitsio/wcslib wrappers (FITSIO.jl, WCS.jl) to their pure-Julia replacements: FITSFiles.jl for FITS I/O and FITSWCS.jl for world coordinate transforms. This is a breaking release (0.3.3 --> 0.4.0) because the accepted input types change from
FITSIO.ImageHDU/FITS/WCS.WCSTransformtoFITSFiles.HDU, vectors of HDUs as returned byFITSFiles.fits, andFITSWCS.WCSTransform.API swap
FITS/ImageHDU/read_header-->fits/HDU;WCSTransform(2; ...)construction -->WCS(2; ...);pix_to_world/world_to_pix-->pixel_to_world/world_to_pixel(both packages use the same 1-based FITS pixel convention, so no coordinate offsets changed).parsers.jlsimplifies considerably. The header-string plumbing (WCS.from_header(read_header(hdu, String))[1]) collapses toWCS(hdu)via FITSWCS's FITSFiles package extension, and the do-block file handling is gone sincefitsreads headers up front and holds no file handles (which also retires the Windows file-locking caveat from the docs).wcs_to_celestial_frameis unchanged: the FITS-standard RADESYS defaulting it relied on from wcslib is now implemented in FITSWCS.jl itself (fix: Follow the FITS standard when RADESYS is absent, and support pv in the keyword constructor FITSWCS.jl#19). The SkyCoords frame dispatch is hoisted out of the per-pixel loop while touching it.Simplified input handling
select_hdu, replacing the FITSIO-era parallelString/Vectormethod pairs and theisa-branching insidereproject. "Parsing" an input is now just FITSFiles for the data and FITSWCS for the transform (data_and_wcs(hdu) = hdu.data, WCS(hdu)).hdu_in/hdu_outare typed via a documented aliasHDUSelector = Union{Integer, AbstractString, Symbol}and now select HDUs by 1-based index orEXTNAME, e.g.,reproject("m101.fits", target, hdu_in = "SCI"), something the FITSIO.jl-based API couldn't do. When you already hold the HDU vector, indexing it yourself (reproject(hdus[2], target)) remains the direct spelling. The keywords exist for the file-name convenience path.shape_outwas silently ignored when the output projection was a file name or HDU vector (reproject(in_file, out_file, shape_out = (1000, 1000))returned a 721×720 array). It is now honored on every path, with a regression test.celestial_frame(wcs), which maps aWCSTransformdirectly to its SkyCoords type (ICRSCoords,FK5Coords{equinox},GalCoords) in one place and raises an informative error for frames without a SkyCoords counterpart (FK4, ITRS). This function is a candidate for upstreaming as a FITSWCS <--> SkyCoords package extension later.Edge handling via Interpolations.jl
pad_edges(which copied the input into a padded array with replicated borders) is replaced byInterpolations.extrapolatewith apadding = Flat()keyword oninterpolator. Sampling semantics are identical for orders 0 and 1; order 2 changes only within the existing test tolerances and now matches the coordinate-clamping approach modern astropyreprojectuses (astropy/reproject#352). This also removes a::Matrixrestriction, so anyAbstractMatrix(e.g., anAstroImage) can flow through the interpolation core.Temporary
[sources]until upstream releases are registeredThe required FITSFiles/FITSWCS changes are unreleased, so the root,
test/, anddocs/projects carry temporary[sources]tables:hdu-size(JuliaAstro/FITSFiles.jl#45)Base.sizeradesys-defaults(JuliaAstro/FITSWCS.jl#19)pvkwargmakie-v0.25makieload/imview/implotviewexamplemain,icweaver#makie-0.25ff/breaking-0.25implotviewrenderingDocumentation overhaul
@extrefagainst the FITSFiles, AstroImages, Julia, and Pkg inventories).load, animview/mosaicside-by-side of the reprojected image and its footprint, and a fullimplotviewpanel with WCS axes, grid, and colorbar rendered by CairoMakie, where the FK5 frame labels come straight from the FITSWCS RADESYS fix flowing through the stack.