Add WCS-driven orientation so thumbnails are consistent across the network - #14
Conversation
…etwork fits_to_img, fits_to_jpg, fits_to_tif, fits_to_zoom_slice_jpg and get_scaled_image take orient='wcs', which puts north up and east left from the frame's CD matrix. The transform is snapped to the nearest 90 degrees, so it is a mirror plus quarter turns and no pixel is resampled. A frame with no usable WCS falls back to the previous fixed vertical flip. The default stays orient='legacy', so no existing caller changes behaviour until it opts in. Also in this commit - needed to get the above under test on currently deployed dependency versions: - Fix _add_label crashing on Pillow >= 10, which removed FreeTypeFont.getsize. - Fix least_squares_line_fit under numpy 2, which no longer converts a 1-element array to a float implicitly. This raised TypeError on every auto-scaled conversion. A rank-deficient fit, which returns no residuals at all, no longer raises either. - Widen the label error handler from IOError, which does not catch what modern Pillow raises, so a font problem degrades to a warning as intended instead of failing the conversion. - get_reduced_dimensionality_data now fills in keywords the data HDU lacks from the primary header, so multi-extension frames expose their WCS and SATURATE. - get_scaled_image reads the frame once rather than twice on the auto-scale path, now that it needs the header for orientation anyway. - Declare python_requires >=3.10 and minimum numpy, astropy and Pillow versions. Floors are the oldest combination the suite passes on; there are deliberately no upper bounds. - Add a test suite and CI across Python 3.10-3.13, plus a row pinning the oldest supported dependencies.
pixel and a frame that has lost its WCS would otherwise be stacked against frames that were rotated. Report an unknown orient the way every other bad argument is reported, rather than raising through fits_to_jpg.
There was a problem hiding this comment.
Pull request overview
Adds an opt-in WCS-derived orientation mode to make generated thumbnails consistent (north up, east left) across instruments/rotations, while preserving the existing default behavior until callers switch over.
Changes:
- Introduces
orient='wcs'|'legacy'plumbing through scaling + conversion entrypoints, including color-stack fallback behavior when channels can’t share one transform. - Improves FITS header handling for multi-extension frames by merging missing primary-header keywords into the data HDU’s header.
- Adds a first test suite and CI/tox configuration covering Python 3.10–3.13 and dependency floor/ceiling behavior (Pillow 10 / NumPy 2 regressions included).
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
tox.ini |
Updates tox to run the new unittest-based test suite across modern Python versions. |
tests/test_scaling.py |
Adds tests for header merging, scaling split, NumPy 2 lstsq behavior, and scaled-image read count/orientation. |
tests/test_orientation.py |
Adds empirical WCS-orientation tests to validate “north up / east left” across rotations and flip combinations. |
tests/test_conversions.py |
Adds end-to-end tests for Pillow 10 label handling and orientation propagation through public conversion APIs. |
tests/helpers.py |
Provides FITS/WCS test fixtures and FITS writers (single-HDU and multi-HDU Sinistro-like). |
tests/__init__.py |
Initializes the tests package. |
setup.py |
Bumps version to 0.5.0, sets python_requires>=3.10, and adds minimum dependency floors; excludes tests from packages. |
requirements.pip |
Removes the previous placeholder requirements file. |
README |
Documents the 0.5.0 WCS-driven orientation behavior and multi-extension header inheritance change. |
fits2image/scaling.py |
Adds orient support to get_scaled_image, splits auto_scale_data, and adds header-only read + header merge logic. |
fits2image/orientation.py |
Introduces WCS/CD-matrix-driven orientation (snapped to nearest 90°) with legacy fallback behavior. |
fits2image/conversions.py |
Threads orient through conversion APIs, adds color-stack orientation selection, and updates label sizing for Pillow 10. |
.github/workflows/run-tests.yml |
Adds CI matrix for Python 3.10–3.13 and oldest-supported dependency pins; runs unittests. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| setup( | ||
| name="fits2image", | ||
| version="0.4.11", | ||
| version="0.5.0", |
There was a problem hiding this comment.
We could be brave and call it 1.0.0
markBowman
left a comment
markBowman
left a comment
There was a problem hiding this comment.
Tried hard to find something to say to show I read it, but it's too damn perfect.
jnation3406
left a comment
jnation3406
left a comment
There was a problem hiding this comment.
I'm not sure about the orient='legacy/wcs' param, but if you can explain with what applications we would want to continue using legacy rather than switch to wcs with a legacy fallback that would help me understand.
Also I noticed you did not add the wcs option to the other method of reducing images, the multi_fits_to_img / quick_scale_img. That new method is currently just used by datalab, but we may want to switch other things to using it as it's more efficient and allows greater flexibility in producing colored composites. Is there a reason not to include the wcs stuff there?
Otherwise it all looks reasonable to me
If any frame in a stack has usable wcs then use that.
|
jnation3406
left a comment
jnation3406
left a comment
There was a problem hiding this comment.
Looks good. Just a few more questions to satiate my own curiosity
fits_to_img,fits_to_jpg,fits_to_tif,fits_to_zoom_slice_jpgandget_scaled_imagetakeorient='wcs', which puts north up and east left from theframe's CD matrix, snapped to the nearest 90 degrees. A frame with no usable WCS
falls back to the fixed vertical flip.
The default for now is
orient='legacy', so nothing moves until a caller opts in. Outputunder the default is identical to 0.4.11.
Tested with a range of real images.
Two things to note:
comes out in the other aspect.
orient='wcs'only when all of its frames resolve to the sametransform, since the channels are combined pixel for pixel. This should always be true anyway.
Also incliuded:
the primary HDU. Sinistro carries its WCS and SATURATE only on the primary, so
quick_scale_imageandmulti_fits_to_imgnow see a SATURATE for those frames.FreeTypeFont.getsizeremoved, which broke every labelledconversion) and numpy 2 (
lstsqresiduals no longer convert to float implicitly).supported dependency set.