Skip to content

fix(wcs)!: Integrate AstroImages.jl with FITSWCS.slice_wcs - #117

Merged
icweaver merged 2 commits into
mainfrom
wcs-slicing
Jul 13, 2026
Merged

fix(wcs)!: Integrate AstroImages.jl with FITSWCS.slice_wcs#117
icweaver merged 2 commits into
mainfrom
wcs-slicing

Conversation

@icweaver

Copy link
Copy Markdown
Member

Closes the loop on the slice_wcs integration discussed in #113 (comment). Depends on JuliaAstro/FITSWCS.jl#17.

Motivation

This PR aims to adopt FITSWCS.slice_wcs to simplify the parent-frame bookkeeping here, and also fix several correctness bugs that were unearthed in the process.

Bugs fixed

  1. Strided slices off by step-1: Legacy mapped slice-local --> parent as p*step + first - 1. Correct is (p-1)*step + first. The formulas agree only when step == 1, so img[1:2:9, :] reported world coordinates one parent pixel off.
  2. Frozen refdims off by one pixel: Dropped axes were fed dim[1] - 1. pixel_to_world(slice; all = true) reported, e.g., FREQ = 1.0e9 where the truth for a slice at parent index 2 is 1.001e9. (Also affected the slice-label values in implot titles.)
  3. parent = true shifted by one pixel: pixel_to_world(img, p; parent = true) equaled raw(p .- 1), disagreeing with the default path on an unsliced image and shifting WCS-grid extents in the plot recipes by one pixel.
  4. Coupled inverses were garbage: world_to_pixel stuffed frozen pixel indices into world coordinate slots before inverting. Harmless only when axes are fully separable. Frozen axes now contribute their exact world values (one forward evaluation).
  5. Underdetermined inverses were silent: Slicing one axis of a celestial lon/lat pair leaves world axes that depend on the dropped pixel axis. world_to_pixel now throws a descriptive ArgumentError (astropy's SlicedLowLevelWCS similarly refuses) instead of returning wrong pixels.
  6. wcsdims lost on every slice: Both rebuild methods defaulted to recomputing wcsdims from the already-sliced dims (DimensionalData no longer routes slicing through rebuildsliced), which broke categorical (Stokes) refdim resolution and name-tracked transposition. rebuild now preserves the parent wcsdims.

New architecture (src/wcs.jl)

  • pixel_to_world: one body for all cases. Expand dims-ordered coords into the full parent frame (kept dims via their lookups, frozen refdims at their parent positions, degenerate header axes at CRPIX), evaluate the full transform, filter to the selected dims (all = true returns every axis).
  • world_to_pixel (default): Delegates to a SlicedWCSTransform (FITSWCS.slice_wcs) describing the current view, guarded by world_keep == pixel_keep.
  • world_to_pixel(parent = true): Unchanged contract for the WCS-grid plot recipes (naxis coordinates, WCS axis order), now with exact frozen-axis world values.
  • Both legacy scatter/gather bodies and the _world_to_pixel worker are deleted. Docstrings now document wcsn / all / parent and the 1-based FITS convention.

⚠️ Breaking / behavior changes

  • world_to_pixel(img, ...) returns ndims(img) slice-local coordinates in dims(img) order (was: naxis parent-frame values), making it the actual inverse of pixel_to_world. Parent-frame output remains available via parent = true.
  • world_to_pixel throws on slices that drop a pixel axis the remaining world axes depend on (was: silently wrong values).
  • parent = true no longer shifted by one pixel. WCS grid overlays now land at their true positions.
  • Strided-slice and frozen-axis coordinates corrected per bugs 1–2 above.

@codecov

codecov Bot commented Jul 11, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.61017% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 56.53%. Comparing base (fe19925) to head (d0291e3).

Files with missing lines Patch % Lines
src/wcs.jl 96.61% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #117      +/-   ##
==========================================
+ Coverage   55.28%   56.53%   +1.24%     
==========================================
  Files           9        9              
  Lines        1069     1056      -13     
==========================================
+ Hits          591      597       +6     
+ Misses        478      459      -19     

☔ 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.

@icweaver
icweaver marked this pull request as ready for review July 11, 2026 08:36
@icweaver
icweaver merged commit 1f1ade2 into main Jul 13, 2026
6 of 7 checks passed
@icweaver
icweaver deleted the wcs-slicing branch July 13, 2026 03:50
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.

1 participant