scriptorron / indi_pylibcamera

INDI libcamera driver made in Python
MIT License
11 stars 3 forks source link

Update FITS header formatting and timestamps #49

Closed cgobat closed 11 months ago

cgobat commented 11 months ago

§4.3.2 of the FITS 4.0 standard says:

If the units of the keyword value are specified in the comment of the header keyword, it is recommended that the units string be enclosed in square brackets (i.e., enclosed by '[' and ']') at the beginning of the comment field

This PR implements this standard format in the FITS file headers created by this driver. This change to the format of the comments should be entirely innocuous.

This PR also includes a minor refactorization of the header construction to use dictionaries instead of lists of tuples. Outward-facing functionality should be unaffected, but dictionary objects feed more naturally into the astropy.io.fits constructors and require less verbosity for the HDU header creation.

Finally, I've switched the header card that was previously DATE-OBS to now be DATE-END (which is the standard keyword for denoting the end of the observation), and added a new DATE-OBS card whose value is calculated by subtracting EXPTIME seconds from DATE-END to determine a nominal observation start time. It is a relatively rough estimate, but I think it is still an improvement over what was there before.

Let me know if there are any changes to this that you'd like to see. Thanks for a great piece of software!

scriptorron commented 11 months ago

Hi Caden,

many thanks for your contribution.

Is the order of the header cards important when constructing the FITS header? I used a list of tuples to force the same order as in the files created by the INDI CCD simulator. When this is not needed I would prefer your dict solution which is much more elegant.

I visually reviewed your changes and do not see an issue. Before merging I would like to test it on my Pi, just to be sure. My plan is to do it this evening. In the next days the indi_pylibcamera will get a new release and your changes will be in there (together with some other improvements I am working on).

Best Regards, Ronald

cgobat commented 11 months ago

Is the order of the header cards important when constructing the FITS header?

Not as far as the FITS standard is concerned, once you get past the first couple required ones (SIMPLE, BITPIX, NAXIS, etc.), which astropy takes care of for us anyway. So, it would not be a big deal if the order of the keywords was different.

However, since Python 3.7, dicts are guaranteed to preserve order, just like lists, so I think we are in the clear either way. 🙂

Before merging I would like to test it on my Pi, just to be sure. My plan is to do it this evening. In the next days the indi_pylibcamera will get a new release

Sounds great! Let me know if anything comes up in testing—thanks for all your work.

scriptorron commented 11 months ago

I fixed some minor issues:

It seems that the "KStars FITS Viewer" has a problem with the "/" in the FITS comments: grafik But other programs (I tried FitsWork, ASTAP and Tenmon) show the comments correctly. It is probably a bug in KStars.

Many thanks again for your 2 PRs!

cgobat commented 11 months ago

Great! Sorry for missing those couple of things and thanks for fixing them. I can open an issue in the KStars repo regarding the / weirdness—that definitely shouldn't be a problem.

Regarding the datetime import: I had done that just to try to cut down on some verbosity in the code. It makes no real difference to me as long as you're okay with the longer lines of code ;)

Thanks!

cgobat commented 11 months ago

Ref KStars bug report #475765

scriptorron commented 11 months ago

Hi Caden,

I have a short question and hope you can help me.

My understanding of the following FITS keywords was:

For instance a physical 1.5µm pixel with 2x binning has PIXSIZE1 = 1.55 and XPIXSZ = 3.1.

I saw now that CCDciel overwrites some of the FITS header. It stores files with:

PIXSIZE1=                  3.1 / [um] Pixel Size X, binned                      
PIXSIZE2=                  3.1 / [um] Pixel Size Y, binned                      
XBINNING=                    2 / Binning factor X                               
YBINNING=                    2 / Binning factor Y                               
XPIXSZ  =                  3.1 / [um] Pixel Size X, binned                      
YPIXSZ  =                  3.1 / [um] Pixel Size Y, binned

Here the PIXSIZE1 and PIXSIZE2 are also the binned pixel sizes. Is that a bug in CCDciel or is my understanding of PIXSIZE1 and PIXSIZE2 wrong?

Regards, Ronald

cgobat commented 11 months ago

I don't think there is any official recommendation or definition for any of those keywords in the FITS standard itself. I did find Diffraction Limited's MaximDL header keyword definitions, which agrees that XPIXSZ and YPIXSZ values should include binning, but maybe that's not super helpful, since it seems like everything already agrees on that one anyway. I couldn't find much in the way of formalized/official descriptions for the use of the PIXSIZE* keywords at all.

So, I'm not sure why CCDciel is doing what it does, since it seems like the de facto convention, at least, aligns with what you've described.

scriptorron commented 11 months ago

Thank you. Maybe this explains why I have trouble with the EKOS plate solver. Every tool seems to handle this in a different way. For instance when debayering the CCDciel image with Siril the PIXSIZE* keywords get deleted and only *PIXSZ = 3.1 and *BINNING = 2 survive. For the debayered image the photometry tool in Siril shows me an effective pixel size of 6.2µm. That's only possible when it takes the binnend *PIXSZ as unbinned pixel size. Really confusing.