-
-
Notifications
You must be signed in to change notification settings - Fork 129
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Preserve fits primary header when writing out spectrum1d without wcs #1105
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1105 +/- ##
==========================================
- Coverage 70.73% 70.05% -0.69%
==========================================
Files 64 65 +1
Lines 4483 4538 +55
==========================================
+ Hits 3171 3179 +8
- Misses 1312 1359 +47 ☔ View full report in Codecov by Sentry. |
You cannot write tables to the PrimaryHDU, the standard forbids anything but IMAGE HDUs there. So yes, to store the data there you need a WCS, even if it's just the simplest case of In principle you could probably get your info written to the Primary header by first creating an empty specutils/specutils/io/default_loaders/wcs_fits.py Lines 265 to 270 in ba0ab68
instead of passing it to |
Ultimately this PR is just a hack around the fact that BinTableHDU cannot be written to the 0th extension, as you mentioned @dhomeier. The only desired behavior (by both what this PR may become and by the existing
This PR is basically the same thing, but it is for the case where there is no 1D WCS and the HDUList needs to be created from scratch. For this reason, I don't see the case for both of these existing - we need a 'tabular' reader/writer that contains the correct logic for headers (and therefore WCS). Theres also the related issue of where header data should be expected to be stored, which is not consistant amongst the reader/writers (#1102) I can get this working to solve the case of 'I have a Spectrum1D without a WCS, and I want to stuff it in a fits file with a primary header', but I think there are some further concerns with this that need to be discussed. |
I probably would not call the way
I am not sure I can follow you here – are you still referring to the
If you are talking about the new loader from this PR and the old |
(this is a suuuuper draft pr, just opening to track work done at Spectro 2023 hack day with @kelle)
The current issue (that we have been trying to wrap our heads around in #1096, #905, and has become clearer to us as we've explored this) is that tabular-fits writer, which converts Spectrum1D into a table and uses Table.write to save to file, does not support writing out to the PrimaryHDU. The alternative writer available for
Spectrum1D
, `wcs1d-fits', sort of does what we want (which is, a primary HDU with a header and a BinTableHDU in the 1st extension with spectrum info) but requires a valid wcs to work (since the initial HDUList is created from this WCS)The goal of this PR is to bypass the tabular-fits writer to enable writing out meta.header info to the primary HDU when there is NO wcs.