From 4524374ee055223cd761d1faeeea1e9eb82c2680 Mon Sep 17 00:00:00 2001 From: Paul Price Date: Fri, 25 Oct 2024 10:23:26 -1000 Subject: [PATCH] fits: ensure long keywords have HIERARCH prepended Otherwise, cfitsio doesn't treat them right. In particular, long string values have the CONTINUE convention messed up, and I've seen it swallow 9 chars from the string value to put in the HIERARCH. --- src/fits.cc | 43 +++++++++++++++++++++++++------------------ tests/test_fits.py | 19 +++++++++++++++++++ 2 files changed, 44 insertions(+), 18 deletions(-) diff --git a/src/fits.cc b/src/fits.cc index 48054f72c..73fe805cd 100644 --- a/src/fits.cc +++ b/src/fits.cc @@ -1006,88 +1006,95 @@ void writeKeyFromProperty(Fits &fits, daf::base::PropertySet const &metadata, st BOOST_CURRENT_FUNCTION % key % upperKey); } std::type_info const &valueType = metadata.typeOf(key); + + // Ensure long keywords have "HIERARCH " prepended; otherwise, cfitsio doesn't treat them right. + std::string keyName = key; + if (keyName.size() > 8 && keyName.rfind("HIERARCH ", 0) != 0) { + keyName = "HIERARCH " + keyName; + } + if (valueType == typeid(bool)) { if (metadata.isArray(key)) { std::vector tmp = metadata.getArray(key); // work around unfortunate specialness of std::vector for (std::size_t i = 0; i != tmp.size(); ++i) { - writeKeyImpl(fits, key.c_str(), static_cast(tmp[i]), comment); + writeKeyImpl(fits, keyName.c_str(), static_cast(tmp[i]), comment); } } else { - writeKeyImpl(fits, key.c_str(), metadata.get(key), comment); + writeKeyImpl(fits, keyName.c_str(), metadata.get(key), comment); } } else if (valueType == typeid(std::uint8_t)) { if (metadata.isArray(key)) { std::vector tmp = metadata.getArray(key); for (std::size_t i = 0; i != tmp.size(); ++i) { - writeKeyImpl(fits, key.c_str(), tmp[i], comment); + writeKeyImpl(fits, keyName.c_str(), tmp[i], comment); } } else { - writeKeyImpl(fits, key.c_str(), metadata.get(key), comment); + writeKeyImpl(fits, keyName.c_str(), metadata.get(key), comment); } } else if (valueType == typeid(int)) { if (metadata.isArray(key)) { std::vector tmp = metadata.getArray(key); for (std::size_t i = 0; i != tmp.size(); ++i) { - writeKeyImpl(fits, key.c_str(), tmp[i], comment); + writeKeyImpl(fits, keyName.c_str(), tmp[i], comment); } } else { - writeKeyImpl(fits, key.c_str(), metadata.get(key), comment); + writeKeyImpl(fits, keyName.c_str(), metadata.get(key), comment); } } else if (valueType == typeid(long)) { if (metadata.isArray(key)) { std::vector tmp = metadata.getArray(key); for (std::size_t i = 0; i != tmp.size(); ++i) { - writeKeyImpl(fits, key.c_str(), tmp[i], comment); + writeKeyImpl(fits, keyName.c_str(), tmp[i], comment); } } else { - writeKeyImpl(fits, key.c_str(), metadata.get(key), comment); + writeKeyImpl(fits, keyName.c_str(), metadata.get(key), comment); } } else if (valueType == typeid(long long)) { if (metadata.isArray(key)) { std::vector tmp = metadata.getArray(key); for (std::size_t i = 0; i != tmp.size(); ++i) { - writeKeyImpl(fits, key.c_str(), tmp[i], comment); + writeKeyImpl(fits, keyName.c_str(), tmp[i], comment); } } else { - writeKeyImpl(fits, key.c_str(), metadata.get(key), comment); + writeKeyImpl(fits, keyName.c_str(), metadata.get(key), comment); } } else if (valueType == typeid(std::int64_t)) { if (metadata.isArray(key)) { std::vector tmp = metadata.getArray(key); for (std::size_t i = 0; i != tmp.size(); ++i) { - writeKeyImpl(fits, key.c_str(), tmp[i], comment); + writeKeyImpl(fits, keyName.c_str(), tmp[i], comment); } } else { - writeKeyImpl(fits, key.c_str(), metadata.get(key), comment); + writeKeyImpl(fits, keyName.c_str(), metadata.get(key), comment); } } else if (valueType == typeid(double)) { if (metadata.isArray(key)) { std::vector tmp = metadata.getArray(key); for (std::size_t i = 0; i != tmp.size(); ++i) { - writeKeyImpl(fits, key.c_str(), tmp[i], comment); + writeKeyImpl(fits, keyName.c_str(), tmp[i], comment); } } else { - writeKeyImpl(fits, key.c_str(), metadata.get(key), comment); + writeKeyImpl(fits, keyName.c_str(), metadata.get(key), comment); } } else if (valueType == typeid(std::string)) { if (metadata.isArray(key)) { std::vector tmp = metadata.getArray(key); for (std::size_t i = 0; i != tmp.size(); ++i) { - writeKeyImpl(fits, key.c_str(), tmp[i], comment); + writeKeyImpl(fits, keyName.c_str(), tmp[i], comment); } } else { - writeKeyImpl(fits, key.c_str(), metadata.get(key), comment); + writeKeyImpl(fits, keyName.c_str(), metadata.get(key), comment); } } else if (valueType == typeid(std::nullptr_t)) { if (metadata.isArray(key)) { // Write multiple undefined values for the same key auto tmp = metadata.getArray(key); for (std::size_t i = 0; i != tmp.size(); ++i) { - writeKeyImpl(fits, key.c_str(), comment); + writeKeyImpl(fits, keyName.c_str(), comment); } } else { - writeKeyImpl(fits, key.c_str(), comment); + writeKeyImpl(fits, keyName.c_str(), comment); } } else { // FIXME: inherited this error handling from fitsIo.cc; need a better option. diff --git a/tests/test_fits.py b/tests/test_fits.py index 0d6ad9ff5..3b89a6357 100644 --- a/tests/test_fits.py +++ b/tests/test_fits.py @@ -168,6 +168,25 @@ def testNamedHeaderNavigate(self): with self.assertRaises(lsst.afw.fits.FitsError): lsst.afw.fits.readMetadata(testfile, hduName="CORDON_BLEAU") + def testReallyLongString(self): + """Check that long keywords have long strings parsed correctly""" + header = PropertyList() + key = "LSST FOO BAR KEYWORD" + longString = ( + "Some very very very really really really REALLY super-duper long string value so we blast past " + "the end of not only the 80 char limit but also the two times 80 char limit and then some more." + ) + + for keyword in (key, "HIERARCH " + key): + header[keyword] = longString + manager = lsst.afw.fits.MemFileManager() + fits = lsst.afw.fits.Fits(manager, "w") + fits.createEmpty() + fits.writeMetadata(header) + fits.closeFile() + new = lsst.afw.fits.Fits(manager, "r").readMetadata(True) + self.assertEqual(new[key], longString) + class TestMemory(lsst.utils.tests.MemoryTestCase): pass