Skip to content

Fix and extend unwrap_wcs_to_fitswcs - #976

Open
samaloney wants to merge 11 commits into
sunpy:mainfrom
samaloney:bugfix-unwrap-preserve-units
Open

Fix and extend unwrap_wcs_to_fitswcs#976
samaloney wants to merge 11 commits into
sunpy:mainfrom
samaloney:bugfix-unwrap-preserve-units

Conversation

@samaloney

@samaloney samaloney commented Sep 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

PR Description

  • Fix bug when used with a WCS with preserve_units=True
  • Fix the crpix update so extents match after rebin
  • Add support to update PC and CD matrices
  • PC
  • CD
  • SIP

Closes #975

AI Assistance Disclosure

AI tools were used for:

  • [ ] Code generation (e.g., when writing an implementation or fixing a bug)
  • [ ] Test/benchmark generation
  • [ ] Documentation (including examples)
  • [ ] Research and understanding
  • [x ] No AI tools were used

Regardless of AI use, the human contributor remains fully responsible for correctness, design choices, licensing compatibility, and long-term maintainability.

samaloney
samaloney commented Sep 27, 2026
View reviewed changes
Comment thread ndcube/wcs/tools.py
@samaloney samaloney changed the title Add failing test Fix unwrap_wcs_to_fitswcs so it works with resample and preserve_units WCSs Sep 27, 2026
@samaloney samaloney mentioned this pull request Sep 27, 2026
Draft
samaloney
samaloney commented Sep 27, 2026
View reviewed changes
Comment thread ndcube/wcs/tests/test_tools.py
Comment on lines +74 to +75
# With preserve_units=True and non-degree celestial units, astropy returns
# cdelt as a read-only copy, so the resample must not modify it in place.

@samaloney samaloney Sep 27, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I've looked at the astropy python and C and I can't see any issues with this but I'm not 100%

@samaloney samaloney changed the title Fix unwrap_wcs_to_fitswcs so it works with resample and preserve_units WCSs Fix and extend unwrap_wcs_to_fitswcs Sep 28, 2026
@samaloney samaloney mentioned this pull request Oct 1, 2026
Draft

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

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

unwrap_wcs_to_fitswcs fails on rebinned cubes when the WCS uses preserve_units=True

1 participant

@samaloney