Skip to content

refactor: Use FITSFiles.jl + FITSWCS.jl - #32

Draft
icweaver wants to merge 8 commits into
mainfrom
fits-wcs
Draft

icweaver wants to merge 8 commits into
mainfrom
fits-wcs

Conversation

@icweaver

@icweaver icweaver commented Jul 23, 2026 •

Copy link
Copy Markdown
Member

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.WCSTransform to FITSFiles.HDU, vectors of HDUs as returned by FITSFiles.fits, and FITSWCS.WCSTransform.

using Reproject, FITSFiles

hdus = fits("gc_msx_e.fits")
result, footprint = reproject(hdus, "gc_2mass_k.fits"; shape_out = (1000, 1000), order = 2)

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.jl simplifies considerably. The header-string plumbing (WCS.from_header(read_header(hdu, String))[1]) collapses to WCS(hdu) via FITSWCS's FITSFiles package extension, and the do-block file handling is gone since fits reads headers up front and holds no file handles (which also retires the Windows file-locking caveat from the docs).
  • wcs_to_celestial_frame is 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

  • HDU selection now defers entirely to FITSFiles.jl's own indexing through a single helper, select_hdu, replacing the FITSIO-era parallel String/Vector method pairs and the isa-branching inside reproject. "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_out are typed via a documented alias HDUSelector = Union{Integer, AbstractString, Symbol} and now select HDUs by 1-based index or EXTNAME, 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.
  • Unifying the output paths surfaced and fixed a latent bug: shape_out was 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.
  • The string-based celestial frame branching is replaced by celestial_frame(wcs), which maps a WCSTransform directly 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 by Interpolations.extrapolate with a padding = Flat() keyword on interpolator. 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 astropy reproject uses (astropy/reproject#352). This also removes a ::Matrix restriction, so any AbstractMatrix (e.g., an AstroImage) can flow through the interpolation core.

Temporary [sources] until upstream releases are registered

The required FITSFiles/FITSWCS changes are unreleased, so the root, test/, and docs/ projects carry temporary [sources] tables:

Package Source Needed for Removable when
FITSFiles hdu-size (JuliaAstro/FITSFiles.jl#45) header precision, HDU vector dispatch, Base.size FITSFiles 0.4.0 registered
FITSWCS radesys-defaults (JuliaAstro/FITSWCS.jl#19) RADESYS defaulting, pv kwarg FITSWCS registered
SkyCoords makie-v0.25 Makie 0.25 compat, scoped CondaPkg deps SkyCoords > 1.7.2 registered
AstroImages (docs) makie load/imview/implotview example AstroImages 0.6 registered
AstroAngles, DimensionalData (docs) main, icweaver#makie-0.25 AstroImages requirements AstroAngles 0.3 / DimensionalData release
Makie, CairoMakie, ComputePipeline (docs) ff/breaking-0.25 implotview rendering Makie 0.25 registered

Documentation overhaul

  • Cross-references use DocumenterInterLinks.jl (@extref against the FITSFiles, AstroImages, Julia, and Pkg inventories).
  • The example is rewritten around AstroImages.jl: load, an imview/mosaic side-by-side of the reprojected image and its footprint, and a full implotview panel 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.

@codecov

codecov Bot commented Jul 23, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 96.96%. Comparing base (45c87ea) to head (276625f).

Files with missing lines Patch % Lines
src/parsers.jl 90.90% 2 Missing ⚠️
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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@barrettp barrettp left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

@icweaver

icweaver commented Jul 23, 2026 •

Copy link
Copy Markdown
Member Author

I think that you are getting ahead of yourself with these changes by assuming that the FITSFiles.jl PR #45 will be approved.

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 [sources] 😅

I've just responded to your comments in #45 to try and express why I think this experiment is worth investigating

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?

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)

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?

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 👍🏾

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants