Tickets/dm 7221

src/table/io/FitsSchemaInputMapper.cc

@@ -36,29 +36,6 @@ struct SetFitsSchemaString {
     std::string const & _v;
 };
 
-void addOldFluxSlotAliases(AliasMap & aliases, std::string const & prefix) {
-    aliases.set(prefix + "_flux", prefix);
-    aliases.set(prefix + "_fluxSigma", prefix + "_err");
-    aliases.set(prefix + "_flag", prefix + "_flags");
-}
-
-void addOldCentroidSlotAliases(AliasMap & aliases, std::string const & prefix) {
-    aliases.set(prefix + "_xSigma", prefix + "_err_xSigma");
-    aliases.set(prefix + "_ySigma", prefix + "_err_ySigma");
-    aliases.set(prefix + "_x_y_Cov", prefix + "_err_x_y_Cov");
-    aliases.set(prefix + "_flag", prefix + "_flags");
-}
-
-void addOldShapeSlotAliases(AliasMap & aliases, std::string const & prefix) {
-    aliases.set(prefix + "_xxSigma", prefix + "_err_xxSigma");
-    aliases.set(prefix + "_yySigma", prefix + "_err_yySigma");
-    aliases.set(prefix + "_xySigma", prefix + "_err_xySigma");
-    aliases.set(prefix + "_xx_yy_Cov", prefix + "_err_xx_yy_Cov");
-    aliases.set(prefix + "_xx_xy_Cov", prefix + "_err_xx_xy_Cov");
-    aliases.set(prefix + "_yy_xy_Cov", prefix + "_err_yy_xy_Cov");
-    aliases.set(prefix + "_flag", prefix + "_flags");
-}
-
 } // anonymous
 
 class FitsSchemaInputMapper::Impl {
@@ -416,6 +393,52 @@ class StandardReader : public FitsColumnReader {
     Key<T> _key;
 };
 
+class AngleReader : public FitsColumnReader {
+public:
+
+    static std::unique_ptr<FitsColumnReader> make(
+        Schema & schema,
+        FitsSchemaItem const & item,
+        FieldBase<afw::geom::Angle> const & base=FieldBase<afw::geom::Angle>()
+    ) {
+        return std::unique_ptr<FitsColumnReader>(new AngleReader(schema, item, base));
+    }
+
+    AngleReader(
+        Schema & schema,
+        FitsSchemaItem const & item,
+        FieldBase<afw::geom::Angle> const & base
+    ) :
+        _column(item.column),
+        _key(schema.addField<afw::geom::Angle>(item.ttype, item.doc, "", base))
+    {
+        // We require an LSST-specific key in the headers before parsing a column
+        // as Angle at all, so we don't need to worry about other units or other
+        // spellings of radians.  We do continue to support no units for backwards
+        // compatibility.
+        if (!item.tunit.empty() && item.tunit != "rad") {
+            throw LSST_EXCEPT(
+                afw::fits::FitsError,
+                "Angle fields must be persisted in radians (TUNIT='rad')."
+            );
+        }
+    }
+
+    virtual void readCell(
+        BaseRecord & record, std::size_t row,
+        afw::fits::Fits & fits,
+        PTR(InputArchive) const & archive
+    ) const {
+        double tmp = 0;
+        fits.readTableScalar(row, _column, tmp);
+        record.set(_key, tmp * afw::geom::radians);
+    }
+
+private:
+    int _column;
+    Key<afw::geom::Angle> _key;
+};
+
 class StringReader : public FitsColumnReader {
 public:
 
@@ -728,7 +751,7 @@ std::unique_ptr<FitsColumnReader> makeColumnReader(
         }
         if (size == 1) {
             if (item.tccls == "Angle") {
-                return StandardReader<Angle>::make(schema, item);
+                return AngleReader::make(schema, item);
             }
             if (item.tccls == "Array") {
                 return StandardReader<Array<double>>::make(schema, item, 1);

src/table/io/FitsWriter.cc

@@ -74,8 +74,9 @@ struct ProcessSchema {
     }
 
     void specialize(SchemaItem<Angle> const & item, int n) const {
-        if (!item.field.getUnits().empty())
-            fits->writeColumnKey("TUNIT", n, item.field.getUnits());
+        // Always write units for Angles as radians (in-memory Angles field don't use the unit attribute,
+        // single Angle abstracts that away).
+        fits->writeColumnKey("TUNIT", n, "rad");
         fits->writeColumnKey("TCCLS", n, "Angle", "Field template used by lsst.afw.table");
     }
 

src/table/io/OutputArchive.cc

@@ -75,7 +75,16 @@ class OutputArchive::Impl {
                 "All catalogs passed to saveCatalog must be created by makeCatalog"
             );
         }
-        iter->getTable()->getMetadata()->add("AR_NAME", name, "Class name for objects stored here");
+        // Add the name of the class to the header so anyone looking at it can
+        // tell what's stored there.  But we don't want to add it multiple times.
+        try {
+            auto names = iter->getTable()->getMetadata()->getArray<std::string>("AR_NAME");
+            if (std::find(names.begin(), names.end(), name) == names.end()) {
+                iter->getTable()->getMetadata()->add("AR_NAME", name, "Class name for objects stored here");
+            }
+        } catch (pex::exceptions::NotFoundError &) {
+            iter->getTable()->getMetadata()->add("AR_NAME", name, "Class name for objects stored here");
+        }
         indexRecord->set(indexKeys.row0, iter->size());
         indexRecord->set(indexKeys.catArchive, catArchive);
         iter->insert(iter->end(), catalog.begin(), catalog.end(), false);

tests/testTableArchives.cc

@@ -16,6 +16,8 @@
 #include "boost/filesystem.hpp"
 #include "Eigen/Core"
 
+#include "lsst/afw/detection/Footprint.h"
+#include "lsst/afw/detection/HeavyFootprint.h"
 #include "lsst/afw/table/io/Persistable.h"
 #include "lsst/afw/table/io/ArchiveIndexSchema.h"
 #include "lsst/afw/table/io/OutputArchive.h"
@@ -642,3 +644,33 @@ BOOST_AUTO_TEST_CASE(ArchiveImporter) {
     lsst::afw::image::Exposure<float> exposure("tests/data/archiveImportTest.fits");
     BOOST_CHECK(!exposure.getPsf());
 }
+
+
+BOOST_AUTO_TEST_CASE(ArchiveMetadata) {
+    // Test that we only write one AR_NAME header key for each class saved in an HDU, not each instance
+    // (see DM-7221).
+    using namespace lsst::afw::table::io;
+    using namespace lsst::afw::fits;
+    using namespace lsst::afw::detection;
+    using namespace lsst::afw::geom;
+    OutputArchive outArchive;
+    outArchive.put(std::make_shared<Footprint>(Box2I(Point2I(2, 3), Point2I(5, 4))));
+    outArchive.put(std::make_shared<Footprint>(Box2I(Point2I(1, 2), Point2I(7, 6))));
+    outArchive.put(std::make_shared<HeavyFootprint<float>>(Footprint(Box2I(Point2I(1, 2), Point2I(7, 6)))));
+    MemFileManager manager;
+    Fits outFits(manager, "w", Fits::AUTO_CHECK);
+    outArchive.writeFits(outFits);
+    outFits.closeFile();
+    Fits inFits(manager, "r", Fits::AUTO_CHECK);
+    inFits.setHdu(3);
+    lsst::daf::base::PropertyList metadata;
+    inFits.readMetadata(metadata);
+    inFits.closeFile();
+    // Get all values corresponding to the AR_NAME key as a list; there
+    // should be exactly two.
+    auto names = metadata.getArray<std::string>("AR_NAME");
+    std::sort(names.begin(), names.end());
+    BOOST_CHECK_EQUAL(names.size(), 2u);
+    BOOST_CHECK_EQUAL(names[0], "Footprint");
+    BOOST_CHECK_EQUAL(names[1], "HeavyFootprintF");
+}

tests/testTableIO.py

@@ -0,0 +1,62 @@
+#!/usr/bin/env python
+from __future__ import absolute_import, division
+
+#
+# LSST Data Management System
+# Copyright 2016 LSST Corporation.
+#
+# This product includes software developed by the
+# LSST Project (http://www.lsst.org/).
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the LSST License Statement and
+# the GNU General Public License along with this program.  If not,
+# see <http://www.lsstcorp.org/LegalNotices/>.
+#
+
+import unittest
+
+import numpy
+import astropy.io.fits
+import lsst.utils.tests
+import lsst.afw.geom
+import lsst.afw.table
+
+
+class TableIoTestCase(lsst.utils.tests.TestCase):
+
+    def testAngleUnitWriting(self):
+        """Test that Angle columns have TUNIT set appropriately,
+        as per DM-7221.
+        """
+        schema = lsst.afw.table.Schema()
+        key = schema.addField("a", type="Angle", doc="angle field")
+        outCat = lsst.afw.table.BaseCatalog(schema)
+        outCat.addNew().set(key, 1.0*lsst.afw.geom.degrees)
+        with lsst.utils.tests.getTempFilePath(".fits") as tmpFile:
+            outCat.writeFits(tmpFile)
+            inFits = astropy.io.fits.open(tmpFile)
+            self.assertEqual(inFits[1].header["TTYPE1"], "a")
+            self.assertEqual(inFits[1].header["TUNIT1"], "rad")
+
+
+class MemoryTester(lsst.utils.tests.MemoryTestCase):
+    pass
+
+
+def setup_module(module):
+    lsst.utils.tests.init()
+
+
+if __name__ == "__main__":
+    lsst.utils.tests.init()
+    unittest.main()