Skip to content

Commit

Permalink
Merge pull request #750 from lsst/tickets/DM-47152
Browse files Browse the repository at this point in the history 
DM-47152: Writing FITS header with long keywords and long strings is broken
  • Loading branch information
@PaulPrice
PaulPrice authored Oct 29, 2024
2 parents fda6b12 + 4524374 commit 33b1fcc
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 18 deletions.
43 changes: 25 additions & 18 deletions src/fits.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<bool> tmp = metadata.getArray<bool>(key);
// work around unfortunate specialness of std::vector<bool>
for (std::size_t i = 0; i != tmp.size(); ++i) {
writeKeyImpl(fits, key.c_str(), static_cast<bool>(tmp[i]), comment);
writeKeyImpl(fits, keyName.c_str(), static_cast<bool>(tmp[i]), comment);
}
} else {
writeKeyImpl(fits, key.c_str(), metadata.get<bool>(key), comment);
writeKeyImpl(fits, keyName.c_str(), metadata.get<bool>(key), comment);
}
} else if (valueType == typeid(std::uint8_t)) {
if (metadata.isArray(key)) {
std::vector<std::uint8_t> tmp = metadata.getArray<std::uint8_t>(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<std::uint8_t>(key), comment);
writeKeyImpl(fits, keyName.c_str(), metadata.get<std::uint8_t>(key), comment);
}
} else if (valueType == typeid(int)) {
if (metadata.isArray(key)) {
std::vector<int> tmp = metadata.getArray<int>(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<int>(key), comment);
writeKeyImpl(fits, keyName.c_str(), metadata.get<int>(key), comment);
}
} else if (valueType == typeid(long)) {
if (metadata.isArray(key)) {
std::vector<long> tmp = metadata.getArray<long>(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<long>(key), comment);
writeKeyImpl(fits, keyName.c_str(), metadata.get<long>(key), comment);
}
} else if (valueType == typeid(long long)) {
if (metadata.isArray(key)) {
std::vector<long long> tmp = metadata.getArray<long long>(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<long long>(key), comment);
writeKeyImpl(fits, keyName.c_str(), metadata.get<long long>(key), comment);
}
} else if (valueType == typeid(std::int64_t)) {
if (metadata.isArray(key)) {
std::vector<std::int64_t> tmp = metadata.getArray<std::int64_t>(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<std::int64_t>(key), comment);
writeKeyImpl(fits, keyName.c_str(), metadata.get<std::int64_t>(key), comment);
}
} else if (valueType == typeid(double)) {
if (metadata.isArray(key)) {
std::vector<double> tmp = metadata.getArray<double>(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<double>(key), comment);
writeKeyImpl(fits, keyName.c_str(), metadata.get<double>(key), comment);
}
} else if (valueType == typeid(std::string)) {
if (metadata.isArray(key)) {
std::vector<std::string> tmp = metadata.getArray<std::string>(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<std::string>(key), comment);
writeKeyImpl(fits, keyName.c_str(), metadata.get<std::string>(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<std::nullptr_t>(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.
Expand Down
19 changes: 19 additions & 0 deletions tests/test_fits.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 33b1fcc

Please sign in to comment.