Skip to content

Start v0.6 migration - #128

Draft
icweaver wants to merge 10 commits into
mainfrom
v0.6
Draft

icweaver wants to merge 10 commits into
mainfrom
v0.6

Conversation

@icweaver

Copy link
Copy Markdown
Member

WIP

icweaver and others added 8 commits July 10, 2026 20:41
…#113)

Swaps the WCSLIB-backed **WCS.jl** dependency for the pure-Julia
[**FITSWCS.jl**](https://github.com/JuliaAstro/FITSWCS.jl) for all World
Coordinate System parsing and pixel↔world transforms. This removes the
C/binary WCSLIB dependency and lets WCS be parsed directly from the FITS
header (no header --> string round-trip).

### Changes

#### Core

- `emptywcs`: `WCSTransform(n)` --> `WCS(n)`.
- `wcsfromheader`: parses the `FITSHeader` directly via FITSWCS's FITSIO
extension instead of serializing to a string.
- `pix_to_world` / `world_to_pix` / `world_to_pix!` now call FITSWCS's
`pixel_to_world` / `world_to_pixel`. The 1-based `CRPIX` convention and
column-per-coordinate batch layout are identical, so the internal offset
arithmetic is unchanged.
- These functions are now owned/exported by AstroImages rather than
re-exported from WCS.jl. **Public API names are preserved.**
- Widened `AstroImage` constructor signatures to
`AbstractVector{<:WCSTransform}` and added a normalizing constructor,
since FITSWCS's `WCSTransform{N, ...}` is parametric (a concrete
`Vector{WCSTransform{2, ...}}` is _not_ `<: Vector{WCSTransform}` under
Julia's invariance).
- Removed dead code (`serializeheader` / `hdrval_repr`), previously only
used to feed `WCS.from_header`.
- WCS coordinate systems are now identified by their **version
character** (`' '` for the primary system, `'A'`–`'Z'` for alternates),
matching the FITS standard and astropy's `WCS(header, key=...)`
replacing the previous integer index.

    | Before          | After             |
    | --------------- | ----------------- |
    | `wcs(img, 1)`   | `wcs(img, ' ')`   |
    | `wcsn = 1`      | `wcsn = ' '`      |
    | `wcsn = 2`      | `wcsn = 'A'`      |

- **Multiple alternate WCS** are parsed via upstream
[`FITSWCS.WCS_all`](JuliaAstro/FITSWCS.jl#14)
- `wcs(img)` now returns a `Dict{Char,WCSTransform}` (was
`Vector{WCSTransform}`), so `keys(wcs(img))` enumerates the systems
present.

#### Docs

- Removed the WCS.jl InterLinks entry; `pix_to_world` / `world_to_pix`
are now
documented in the AstroImages `@docs` block (added a docstring for
`world_to_pix`).
- Updated prose references (WCS.jl --> FITSWCS.jl).

#### Tests

- `test/general.jl` multi-WCS test is now self-contained: asserts system
count + `ctype` and a pixel→world→pixel round-trip.
- `test/plots.jl`: `WCSTransform(2; ...)` --> `WCS(2; ...)`.

#### Dependencies

| Change | Details |
| --- | --- |
| ➖ Removed | `WCS` from package / test / docs `Project.toml` |
| ➕ Added | `FITSWCS` (via `[sources]` --> GitHub repo until registered)
|
| ⬆️  Version | `0.5.1` --> **`0.6.0`** |

> [!NOTE]
> WCS.jl still appears transitively via `Reproject.jl` in the **docs**
environment only. It is no longer a direct dependency of AstroImages,
the test env, or the docs env.

### Compatibility

- Public API (`wcs`, `pix_to_world`, `world_to_pix`, `world_to_pix!`,
`WCSTransform`) is unchanged.
- `wcs(img)` now returns FITSWCS `WCSTransform` objects. Field access
used by AstroImages (`.naxis`, `.ctype`, `.cunit`, `.crval`, `.radesys`)
is preserved.

> [!WARNING]
> WCS.jl-specific helpers (`to_header`, `from_header`, `WCS.HDR_ALL`)
are no longer available, and the `WCSTransform` type itself changes,
hence the minor (`0.6.0`) version bump.
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.
1. **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.)
1. **`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.
1. **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).
1. **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.
1. **`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.
…120)

**Before:**

```julia-repl
julia> img["KEY"] = nothing
ERROR: MethodError: no method matching FITSFiles.Card(::String, ::Nothing)
The type `FITSFiles.Card` exists, but no method is defined for this combination of argument types when trying to construct it.

Closest candidates are:
  FITSFiles.Card(::Any, ::Any, ::Any, ::Any, ::Any)
   @ FITSFiles ~/.julia/packages/FITSFiles/0MEHx/src/card.jl:232
  FITSFiles.Card(::S; ...) where S<:AbstractString
   @ FITSFiles ~/.julia/packages/FITSFiles/0MEHx/src/card.jl:237
  FITSFiles.Card(::S, ::V; ...) where {S<:AbstractString, V<:Union{Missing, AbstractString, Number}}
   @ FITSFiles ~/.julia/packages/FITSFiles/0MEHx/src/card.jl:237
  ...
```

**After:**

```julia-repl
julia> img["KEY"] = nothing
```
Now we can do `img = load(myfile.fits; scale = false)` to read the data
as exactly stored on disk
… on any header write (#126)

`setindex!` on an `AstroImage` header key previously consulted a
~250-line `WCS_HEADERS_TEMPLATES` table (expanded into an exact-match
`Set`) to decide whether to mark the cached `WCSTransform` stale. Such a
filter can't be complete. WCS keywords span Papers I–IV plus legacy
conventions, with unbounded axis indices and `A`–`Z` alternate-system
suffixes, and the table had real holes: the expansion only covered axis
indices 1–4 and lowercase alternate suffixes, so e.g. `img["CRPIX5"] =
3.0` or `img["CRPIX1A"] = 3.0` silently left a stale transform in use.

Now any header write marks the cached WCS stale, and it is lazily
re-parsed on the next `wcs()` access. Re-parsing is cheap and header
writes are rare, so the extra invalidations cost effectively nothing,
while false negatives are gone by construction. The staleness regression
test now covers the previously-missed key classes.

Breaking only in the internal sense: the unexported
`WCS_HEADERS_TEMPLATES`/`WCS_HEADERS` constants are gone, and cache
invalidation is now unconditional rather than keyword-filtered.
@icweaver
icweaver marked this pull request as draft July 29, 2026 20:55
@codecov

codecov Bot commented Jul 29, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.73171% with 19 lines in your changes missing coverage. Please review.
✅ Project coverage is 61.92%. Comparing base (4edacd2) to head (9737c1b).

Files with missing lines Patch % Lines
src/io.jl 85.48% 9 Missing ⚠️
src/wcs.jl 92.95% 5 Missing ⚠️
src/AstroImages.jl 93.65% 4 Missing ⚠️
src/plot-recipes.jl 88.88% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #128      +/-   ##
==========================================
+ Coverage   53.68%   61.92%   +8.24%     
==========================================
  Files          10        9       -1     
  Lines        1127     1061      -66     
==========================================
+ Hits          605      657      +52     
+ Misses        522      404     -118     

☔ 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 changed the title release: v0.6 Start v0.6 migration Jul 29, 2026
@icweaver
icweaver added this pull request to stack #142 October 2, 2026 19:59

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