WorldWideTelescope / toasty

Break large images into "tile pyramids", with a focus on the all-sky TOAST format.
https://toasty.readthedocs.io/
MIT License
0 stars 4 forks source link

Allow astropy to fix non-standard FITS headers #73

Closed imbasimba closed 2 years ago

imbasimba commented 2 years ago

Warnings are emitted by astropy if any header changes are made.

Not allowing astropy to try fix the wcs can cause ugly crashes for some FITS files, like these.

Here is an example error if fixing is not allowed:

astropy.wcs._wcs.SingularMatrixError: ERROR 3 in wcsset() at line 2739 of file cextern\wcslib\C\wcs.c:
Linear transformation matrix is singular.
ERROR 3 in linset() at line 718 of file cextern\wcslib\C\lin.c:
PCi_ja matrix is singular.

@pkgw Are you aware of why WCS fixing has been set to False? I've not seen anything obvious breaking by allowing fixing, but maybe there is or was some valid reason for introducing this limitation?

codecov[bot] commented 2 years ago

Codecov Report

Merging #73 (de7aa7b) into master (218a0f6) will not change coverage. The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #73   +/-   ##
=======================================
  Coverage   75.05%   75.05%           
=======================================
  Files          23       23           
  Lines        3360     3360           
=======================================
  Hits         2522     2522           
  Misses        838      838           
Impacted Files Coverage Δ
toasty/collection.py 56.57% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 218a0f6...de7aa7b. Read the comment docs.

pkgw commented 2 years ago

Hmm, no, I don't recall why exactly I put the fix=False there. I think that maybe I thought that it would prevent the annoying warnings from being reported, but not actually affect the parsing of the data? I agree that we should remove it.