Repository navigation
Bad checksum not being overwritten on call to "writeto" #5902
Description
Astropy v2.0
Test files used:
io/fits/tests/data/test0.fits- ImageHDU extensionsio/fits/tests/data/checksum.fits- A BinTableHDU extension
While working on #5874 I came across this :-
- I modified the
CHECKSUMheader ofchecksum.fitsand addedCHECKSUMtotest0.fitsand modified it'sCHECKSUMheader too. (i.e. 16 letter long garbage value). - Opened and overwrote both the files. Correct
CHECKSUMs were written only fortest0.fits
This corrected the Garbage checksums.
>>> hlist = fits.open('./tests/data/test0.fits')
>>> hlist.writeto('./tests/data/test0.fits', checksum='standard', overwrite=True, output_verify='ignore')
>>> hlist.close()
and this didn't.
>>> hlist = fits.open('./tests/data/checksum.fits')
>>> hlist.writeto('./tests/data/checksum.fits', checksum='standard', overwrite=True, output_verify='ignore')
>>> hlist.close()
The above one works if something like -
>>> hlist = fits.open('./tests/data/checksum.fits', checksum=True) is used.
Is this because of different types of extensions that are present?
On digging a bit this is happening because hdu._header._modified is False for checksum.fits case.
https://github.com/astropy/astropy/blob/master/astropy/io/fits/hdu/base.py#L522
Activity
mohanagr commentedon Apr 28, 2017
@pllim @Cadair @saimn Any updates for this?
P.S. Apologies for tagging you all. Don't quite know who maintains what.
pllim commentedon Apr 28, 2017
Trying my luck on additional @embray and @MSeifert04 .
cadairis not involved in FITS and I have little to say about this topic.MSeifert04 commentedon Apr 28, 2017
What would be the correct behaviour here? I mean
output_verifyisFalseand thechecksumparameter was "sort of"True. However thechecksumparameter only says "When True adds both DATASUM and CHECKSUM cards to the headers of all HDU’s written to the file.", does that mean it should always overwrite the existing DATASUM and CHECKSUM or just add them if not present?I mean the test case is a bit theoretical: If you explicitly garbage the CHECKSUM and don't check it when reading the file what's the point about "correcting" it when writing the file again?
The documentation of
checksuminfits.openis this:That actually means (in my opinion) that when you suspect a "garbage" checksum you should provide
checksum='remove'when reading the file andchecksum=Truewhen writing it again.embray commentedon May 3, 2017
I'd have to take a closer look, but it's a known (to me) issue that checksum handling is confusing. I believe there may already be some open tickets about this either here or in the old PyFITS repo. Which is not to say the behavior isn't deliberate. It's just confusing.
embray commentedon May 3, 2017
I think this means it should overwrite. DATASUM and CHECKSUM are two of those "special" header keywords that nobody should ever touch manually without very good reason to.
mohanagr commentedon May 3, 2017
saimn commentedon Sep 25, 2017
Fixed by #6571