Merge pull request #446 from lsst/tickets/DM-18864

DM-18864: Now support UNDEF in FITS headers
lsst · Apr 3, 2019 · f7766db · f7766db
2 parents c7a4417 + 33f9b24
commit f7766db
Show file tree

Hide file tree

Showing 7 changed files with 152 additions and 41 deletions.
diff --git a/.gitattributes b/.gitattributes
@@ -0,0 +1 @@
+*.fits binary
diff --git a/src/fits.cc b/src/fits.cc
@@ -70,6 +70,8 @@ std::string makeLimitedFitsHeaderImpl(std::vector<std::string> const &paramNames
             out += (boost::format("%#20.17G") % metadata.get<double>(name)).str();
         } else if (type == typeid(float)) {
             out += (boost::format("%#20.15G") % metadata.get<float>(name)).str();
+        } else if (type == typeid(std::nullptr_t)) {
+            out += " ";
         } else if (type == typeid(std::string)) {
             out += "'" + metadata.get<std::string>(name) + "'";
             if (out.size() > 80) {
@@ -596,6 +598,12 @@ void writeKeyImpl(Fits &fits, char const *key, T const &value, char const *comme
                    const_cast<T *>(&value), const_cast<char *>(comment), &fits.status);
 }
 
+void writeKeyImpl(Fits &fits, char const *key, char const *comment) {
+    // Write a key with an undefined value
+    fits_write_key_null(reinterpret_cast<fitsfile *>(fits.fptr), const_cast<char *>(key),
+                        const_cast<char *>(comment), &fits.status);
+}
+
 void writeKeyImpl(Fits &fits, char const *key, std::string const &value, char const *comment) {
     if (strncmp(key, "COMMENT", 7) == 0) {
         fits_write_comment(reinterpret_cast<fitsfile *>(fits.fptr), const_cast<char *>(value.c_str()),
@@ -821,10 +829,54 @@ class MetadataIterationFunctor : public HeaderIterationFunctor {
 
     template <typename T>
     void add(std::string const &key, T value, std::string const &comment) {
+        // PropertyList/Set can not support array items where some elements are
+        // defined and some undefined. If we are adding defined value where
+        // previously we have an undefined value we must use set instead.
+        if (list) {
+            if (list->exists(key) && list->isUndefined(key)) {
+                LOGLS_WARN("afw.fits",
+                           boost::format("In %s, replacing undefined value for key '%s'.") %
+                                         BOOST_CURRENT_FUNCTION % key);
+                list->set(key, value, comment);
+            } else {
+                list->add(key, value, comment);
+            }
+        } else {
+            if (set->exists(key) && set->isUndefined(key)) {
+                LOGLS_WARN("afw.fits",
+                           boost::format("In %s, replacing undefined value for key '%s'.") %
+                                         BOOST_CURRENT_FUNCTION % key);
+                set->set(key, value);
+            } else {
+                set->add(key, value);
+            }
+        }
+    }
+
+    void add(std::string const &key, std::string const &comment) {
+        // If this undefined value is adding to a pre-existing key that has
+        // a defined value we must skip the add so as not to break
+        // PropertyList/Set.
         if (list) {
-            list->add(key, value, comment);
+            if (list->exists(key) && !list->isUndefined(key)) {
+                // Do nothing. Assume the previously defined value takes
+                // precedence.
+                LOGLS_WARN("afw.fits",
+                           boost::format("In %s, dropping undefined value for key '%s'.") %
+                                         BOOST_CURRENT_FUNCTION % key);
+            } else {
+                list->add(key, nullptr, comment);
+            }
         } else {
-            set->add(key, value);
+            if (set->exists(key) && !set->isUndefined(key)) {
+                // Do nothing. Assume the previously defined value takes
+                // precedence.
+                LOGLS_WARN("afw.fits",
+                           boost::format("In %s, dropping undefined value for key '%s'.") %
+                                         BOOST_CURRENT_FUNCTION % key);
+            } else {
+                set->add(key, nullptr);
+            }
         }
     }
 
@@ -879,7 +931,11 @@ void MetadataIterationFunctor::operator()(std::string const &key, std::string co
     } else if (key == "COMMENT" && !(strip && boost::regex_match(comment, fitsDefinitionCommentRegex))) {
         add(key, comment, "");
     } else if (value.empty()) {
-        // do nothing for empty values
+        // do nothing for empty values that are comments
+        // Otherwise write null value to PropertySet
+        if (key != "COMMENT") {
+            add(key, comment);
+        }
     } else {
         throw LSST_EXCEPT(
                 afw::fits::FitsError,
@@ -963,6 +1019,16 @@ void writeKeyFromProperty(Fits &fits, daf::base::PropertySet const &metadata, st
         } else {
             writeKeyImpl(fits, key.c_str(), metadata.get<std::string>(key), comment);
         }
+    } else if (valueType == typeid(nullptr_t)) {
+        if (metadata.isArray(key)) {
+            // Write multiple undefined values for the same 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(), comment);
+            }
+        } else {
+            writeKeyImpl(fits, key.c_str(), comment);
+        }
     } else {
         // FIXME: inherited this error handling from fitsIo.cc; need a better option.
         LOGLS_WARN("afw.writeKeyFromProperty",

diff --git a/src/geom/detail/frameSetUtils.cc b/src/geom/detail/frameSetUtils.cc
@@ -278,6 +278,14 @@ std::shared_ptr<daf::base::PropertyList> getPropertyListFromFitsChan(ast::FitsCh
                 setMetadataFromFoundValue(*metadata, cardName, foundValue, cardComment);
                 break;
             }
+            case ast::CardType::UNDEF: {
+                if (cardComment.empty()) {
+                    metadata->set(cardName, nullptr);
+                } else {
+                    metadata->set(cardName, nullptr, cardComment);
+                }
+                break;
+            }
             case ast::CardType::CONTINUE: {
                 auto foundValue = fitsChan.getFitsCN();
                 setMetadataFromFoundValue(*metadata, cardName, foundValue, cardComment);
@@ -287,8 +295,7 @@ std::shared_ptr<daf::base::PropertyList> getPropertyListFromFitsChan(ast::FitsCh
                 // Drop HISTORY and COMMENT cards
                 break;
             case ast::CardType::COMPLEXF:
-            case ast::CardType::COMPLEXI:
-            case ast::CardType::UNDEF: {
+            case ast::CardType::COMPLEXI: {
                 // PropertyList supports neither complex numbers nor cards with no value
                 std::ostringstream os;
                 os << "Card " << cardNum << " with name \"" << cardName << "\" has type "