Merge pull request #558 from lsst/tickets/DM-28368

DM-28368: Fix id check so that all components can use readComponent.
lsst · Jan 19, 2021 · 7bed000 · 7bed000
2 parents d9c33f4 + 30c3630
commit 7bed000
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 2 deletions.
diff --git a/include/lsst/afw/image/TransmissionCurve.h b/include/lsst/afw/image/TransmissionCurve.h
@@ -55,6 +55,12 @@ namespace image {
  *  at each position it is evaluated at).  Other classes and functions using
  *  TransmissionCurves should of course document the flux units and/or
  *  normalization expected/provided.
+ *
+ * @note TransmissionCurve is an interface to various C++-only implementation
+ * classes. Because these classes are not mapped to Python, some references to
+ * a TransmissionCurve object may appear in Python as a table::io::Persistable
+ * or typehandling::Storable instead. The best way to avoid this at present is
+ * to call functions that, in C++, explicitly return a TransmissionCurve.
  */
 class TransmissionCurve : public table::io::PersistableFacade<TransmissionCurve>,
                           public typehandling::Storable,

diff --git a/src/image/ExposureFitsReader.cc b/src/image/ExposureFitsReader.cc
@@ -401,6 +401,10 @@ class ExposureFitsReader::ArchiveReader {
      *
      * @throws pex::exceptions::NotFoundError Thrown if the component is
      *     registered in the file metadata but could not be found.
+     *
+     * @note When accessing from python, components with derived subclasses, such
+     *       as ``TransmissionCurve``, are not properly type converted and thus
+     *       the specialized reader must be used instead of ``readComponent``.
      */
     // This method takes a string instead of a strongly typed Key because
     // readExtraComponents() gets its keys from the FITS metadata.
@@ -411,7 +415,7 @@ class ExposureFitsReader::ArchiveReader {
             return nullptr;
         }
 
-        if (_extraIds.count(c) > 0) {
+        if (_genericIds.count(c) > 0) {
             int archiveId = _genericIds.at(c);
             return _archive.get<T>(archiveId);
         } else {

diff --git a/tests/test_readers.py b/tests/test_readers.py
@@ -32,7 +32,7 @@
 from lsst.afw.image import (Image, Mask, Exposure, LOCAL, PARENT, MaskPixel, VariancePixel,
                             ImageFitsReader, MaskFitsReader, MaskedImageFitsReader, ExposureFitsReader,
                             Filter, FilterLabel, PhotoCalib, ApCorrMap, VisitInfo, TransmissionCurve,
-                            CoaddInputs)
+                            CoaddInputs, ExposureInfo)
 from lsst.afw.image.utils import defineFilter
 from lsst.afw.detection import GaussianPsf
 from lsst.afw.cameraGeom.testUtils import DetectorWrapper
@@ -182,18 +182,35 @@ def checkExposureFitsReader(self, exposureIn, fileName, dtypesOut):
         self.assertEqual(exposureIn.getMetadata().toDict(), reader.readMetadata().toDict())
         self.assertWcsAlmostEqualOverBBox(exposureIn.getWcs(), reader.readWcs(), self.bbox,
                                           maxDiffPix=0, maxDiffSky=0*degrees)
+        self.assertWcsAlmostEqualOverBBox(exposureIn.getWcs(),
+                                          reader.readComponent(ExposureInfo.KEY_WCS),
+                                          self.bbox,
+                                          maxDiffPix=0, maxDiffSky=0*degrees)
         self.assertEqual(exposureIn.getFilter(), reader.readFilter())
         self.assertEqual(exposureIn.getFilterLabel(), reader.readFilterLabel())
+        self.assertEqual(exposureIn.getFilterLabel(),
+                         reader.readComponent(ExposureInfo.KEY_FILTER))
         self.assertEqual(exposureIn.getPhotoCalib(), reader.readPhotoCalib())
+        self.assertEqual(exposureIn.getPhotoCalib(),
+                         reader.readComponent(ExposureInfo.KEY_PHOTO_CALIB))
         self.assertImagesEqual(exposureIn.getPsf().computeImage(), reader.readPsf().computeImage())
+        self.assertImagesEqual(exposureIn.getPsf().computeImage(),
+                               reader.readComponent('PSF').computeImage())
         self.assertEqual(exposureIn.getInfo().getValidPolygon(), reader.readValidPolygon())
+        self.assertEqual(exposureIn.getInfo().getValidPolygon(),
+                         reader.readComponent(ExposureInfo.KEY_VALID_POLYGON))
         self.assertCountEqual(exposureIn.getInfo().getApCorrMap(), reader.readApCorrMap())
+        self.assertCountEqual(exposureIn.getInfo().getApCorrMap(),
+                              reader.readComponent(ExposureInfo.KEY_AP_CORR_MAP))
         self.assertEqual(exposureIn.getInfo().getVisitInfo().getExposureTime(),
                          reader.readVisitInfo().getExposureTime())
         point = Point2D(2.3, 3.1)
         wavelengths = np.linspace(4000, 5000, 5)
         self.assertFloatsEqual(exposureIn.getInfo().getTransmissionCurve().sampleAt(point, wavelengths),
                                reader.readTransmissionCurve().sampleAt(point, wavelengths))
+        # Note: readComponent(ExposureInfo.KEY_TRANSMISSION_CURVE) returns a generic Storable
+        # rather than a TransmissionCurve object.
+
         # Because we persisted the same instances, we should get back the same
         # instances for *archive* components, and hence equality comparisons
         # should work even if it just amounts to C++ pointer equality.