Merge pull request #19 from lsst/tickets/DM-19331

DM-19331: Add detector_unique_name property and fix Subaru names

README.md

@@ -11,3 +11,6 @@ This package was developed by the Large Synoptic Survey Telescope
 Data Management team.
 
 It does not depend on any LSST software.
+
+Documentation: <https://astro-metadata-translator.lsst.io>
+Source: <https://github.com/lsst/astro_metadata_translator>

python/astro_metadata_translator/properties.py

@@ -44,8 +44,12 @@
               "boresight_rotation_coord": ("Coordinate frame of the instrument rotation angle"
                                            " (options: sky, unknown).", "str"),
               "detector_num": ("Unique (for instrument) integer identifier for the sensor.", "int"),
-              "detector_name": ("Name of the detector within the instrument (might not be unique).",
+              "detector_name": ("Name of the detector within the instrument (might not be unique"
+                                " if there are detector groups).",
                                 "str"),
+              "detector_unique_name": ("Unique name of the detector within the focal plane, generally"
+                                       " combining detector_group with detector_name.",
+                                       "str"),
               "detector_serial": ("Serial number/string associated with this detector.", "str"),
               "detector_group": ("Collection name of which this detector is a part. "
                                  "Can be None if there are no detector groupings.", "str"),

python/astro_metadata_translator/translator.py

@@ -670,6 +670,38 @@ def _join_keyword_values(self, keywords, delim="+"):
 
         return joined
 
+    @cache_translation
+    def to_detector_unique_name(self):
+        """Return a unique name for the detector.
+
+        Base class implementation attempts to combine ``detector_name`` with
+        ``detector_group``.  Group is only used if not `None`.
+
+        Can be over-ridden by specialist translator class.
+
+        Returns
+        -------
+        name : `str`
+            ``detector_group``_``detector_name`` if ``detector_group`` is
+            defined, else the ``detector_name`` is assumed to be unique.
+            If neither return a valid value an exception is raised.
+
+        Raises
+        ------
+        NotImplementedError
+            Raised if neither detector_name nor detector_group is defined.
+        """
+        name = self.to_detector_name()
+        group = self.to_detector_group()
+
+        if group is None and name is None:
+            raise NotImplementedError("Can not determine unique name from detector_group and detector_name")
+
+        if group is not None:
+            return f"{group}_{name}"
+
+        return name
+
 
 def _make_abstract_translator_method(property, doc, return_type):
     """Create a an abstract translation method for this property.
@@ -710,9 +742,17 @@ def to_property(self):
 # poorly with the metaclass automatically generating methods from
 # _trivialMap and _constMap.
 
+# Allow for concrete translator methods to exist in the base class
+# These translator methods can be defined in terms of other properties
+CONCRETE = set()
+
 for name, description in PROPERTIES.items():
-    setattr(MetadataTranslator, f"to_{name}",
-            abstractmethod(_make_abstract_translator_method(name, *description)))
+    method = f"to_{name}"
+    if not MetadataTranslator.defined_in_this_class(method):
+        setattr(MetadataTranslator, f"to_{name}",
+                abstractmethod(_make_abstract_translator_method(name, *description)))
+    else:
+        CONCRETE.add(method)
 
 
 class StubTranslator(MetadataTranslator):
@@ -728,11 +768,15 @@ class StubTranslator(MetadataTranslator):
     pass
 
 
-def _make_stub_translator_method(property, doc, return_type):
-    """Create a an stub translation method for this property.
+def _make_forwarded_stub_translator_method(cls, property, doc, return_type):
+    """Create a stub translation method for this property that calls the
+    base method and catches `NotImplementedError`.
 
     Parameters
     ----------
+    cls : `class`
+        Class to use when referencing `super()`.  This would usually be
+        `StubTranslator`.
     property : `str`
         Name of the translator for property to be created.
     doc : `str`
@@ -745,25 +789,38 @@ def _make_stub_translator_method(property, doc, return_type):
     m : `function`
         Stub translator method for this property.
     """
+    method = f"to_{property}"
+
     def to_stub(self):
+        parent = getattr(super(cls, self), method, None)
+        try:
+            if parent is not None:
+                return parent()
+        except NotImplementedError:
+            pass
+
         warnings.warn(f"Please implement translator for property '{property}' for translator {self}",
                       stacklevel=3)
         return None
 
-    to_stub.__doc__ = f"""Unimplemented translator for {property}.
+    to_stub.__doc__ = f"""Unimplemented forwarding translator for {property}.
 
     {doc}
 
-    Issues a warning reminding the implementer to override this method.
+    Calls the base class translation method and if that fails with
+    `NotImplementedError` issues a warning reminding the implementer to
+    override this method.
 
     Returns
     -------
-    {property} : `None`
+    {property} : `None` or `{return_type}`
         Always returns `None`.
     """
     return to_stub
 
 
-# Create stub translation methods
+# Create stub translation methods for each property.  These stubs warn
+# rather than fail and should be overridden by translators.
 for name, description in PROPERTIES.items():
-    setattr(StubTranslator, f"to_{name}", _make_stub_translator_method(name, *description))
+    setattr(StubTranslator, f"to_{name}", _make_forwarded_stub_translator_method(StubTranslator,
+                                                                                 name, *description))

python/astro_metadata_translator/translators/decam.py

@@ -36,7 +36,7 @@ class DecamTranslator(FitsTranslator):
 
     _const_map = {"boresight_rotation_angle": Angle(float("nan")*u.deg),
                   "boresight_rotation_coord": "unknown",
-                  "detector_group": None}
+                  }
 
     _trivial_map = {"exposure_time": ("EXPTIME", dict(unit=u.s)),
                     "dark_time": ("DARKTIME", dict(unit=u.s)),
@@ -46,7 +46,7 @@ class DecamTranslator(FitsTranslator):
                     "science_program": "PROPID",
                     "detector_num": "CCDNUM",
                     "detector_serial": "DETECTOR",
-                    "detector_name": "DETPOS",
+                    "detector_unique_name": "DETPOS",
                     "telescope": ("TELESCOP", dict(default="CTIO 4.0-m telescope")),
                     "instrument": ("INSTRUME", dict(default="DECam")),
                     # Ensure that reasonable values are always available
@@ -58,6 +58,20 @@ class DecamTranslator(FitsTranslator):
                                  default=771.611, minimum=700., maximum=850.)),
                     }
 
+    # Unique detector names are currently not used but are read directly from
+    # header.
+    # The detector_group could be N or S with detector_name corresponding
+    # to the number in that group.
+    detector_names = {
+        1: 'S29', 2: 'S30', 3: 'S31', 4: 'S25', 5: 'S26', 6: 'S27', 7: 'S28', 8: 'S20', 9: 'S21',
+        10: 'S22', 11: 'S23', 12: 'S24', 13: 'S14', 14: 'S15', 15: 'S16', 16: 'S17', 17: 'S18',
+        18: 'S19', 19: 'S8', 20: 'S9', 21: 'S10', 22: 'S11', 23: 'S12', 24: 'S13', 25: 'S1', 26: 'S2',
+        27: 'S3', 28: 'S4', 29: 'S5', 30: 'S6', 31: 'S7', 32: 'N1', 33: 'N2', 34: 'N3', 35: 'N4',
+        36: 'N5', 37: 'N6', 38: 'N7', 39: 'N8', 40: 'N9', 41: 'N10', 42: 'N11', 43: 'N12', 44: 'N13',
+        45: 'N14', 46: 'N15', 47: 'N16', 48: 'N17', 49: 'N18', 50: 'N19', 51: 'N20', 52: 'N21',
+        53: 'N22', 54: 'N23', 55: 'N24', 56: 'N25', 57: 'N26', 58: 'N27', 59: 'N28', 60: 'N29',
+        62: 'N31'}
+
     @classmethod
     def can_translate(cls, header, filename=None):
         """Indicate whether this translation class can translate the
@@ -203,3 +217,15 @@ def to_detector_exposure_id(self):
         if exposure_id is None:
             return None
         return int("{:07d}{:02d}".format(exposure_id, self.to_detector_num()))
+
+    @cache_translation
+    def to_detector_group(self):
+        # Docstring will be inherited. Property defined in properties.py
+        name = self.to_detector_unique_name()
+        return name[0]
+
+    @cache_translation
+    def to_detector_name(self):
+        # Docstring will be inherited. Property defined in properties.py
+        name = self.to_detector_unique_name()
+        return name[1:]

python/astro_metadata_translator/translators/hsc.py

@@ -40,7 +40,6 @@ class HscTranslator(SuprimeCamTranslator):
     """Hard wire HSC even though modern headers call it Hyper Suprime-Cam"""
 
     _trivial_map = {"detector_serial": "T_CCDSN",
-                    "detector_name": "T_CCDSN",  # T_CCDID seems to always be undefined
                     }
     """One-to-one mappings"""
 
@@ -56,6 +55,121 @@ class HscTranslator(SuprimeCamTranslator):
                                 114: 108,
                                 }
 
+    _DETECTOR_NUM_TO_UNIQUE_NAME = [
+        '1_53',
+        '1_54',
+        '1_55',
+        '1_56',
+        '1_42',
+        '1_43',
+        '1_44',
+        '1_45',
+        '1_46',
+        '1_47',
+        '1_36',
+        '1_37',
+        '1_38',
+        '1_39',
+        '1_40',
+        '1_41',
+        '0_30',
+        '0_29',
+        '0_28',
+        '1_32',
+        '1_33',
+        '1_34',
+        '0_27',
+        '0_26',
+        '0_25',
+        '0_24',
+        '1_00',
+        '1_01',
+        '1_02',
+        '1_03',
+        '0_23',
+        '0_22',
+        '0_21',
+        '0_20',
+        '1_04',
+        '1_05',
+        '1_06',
+        '1_07',
+        '0_19',
+        '0_18',
+        '0_17',
+        '0_16',
+        '1_08',
+        '1_09',
+        '1_10',
+        '1_11',
+        '0_15',
+        '0_14',
+        '0_13',
+        '0_12',
+        '1_12',
+        '1_13',
+        '1_14',
+        '1_15',
+        '0_11',
+        '0_10',
+        '0_09',
+        '0_08',
+        '1_16',
+        '1_17',
+        '1_18',
+        '1_19',
+        '0_07',
+        '0_06',
+        '0_05',
+        '0_04',
+        '1_20',
+        '1_21',
+        '1_22',
+        '1_23',
+        '0_03',
+        '0_02',
+        '0_01',
+        '0_00',
+        '1_24',
+        '1_25',
+        '1_26',
+        '1_27',
+        '0_34',
+        '0_33',
+        '0_32',
+        '1_28',
+        '1_29',
+        '1_30',
+        '0_41',
+        '0_40',
+        '0_39',
+        '0_38',
+        '0_37',
+        '0_36',
+        '0_47',
+        '0_46',
+        '0_45',
+        '0_44',
+        '0_43',
+        '0_42',
+        '0_56',
+        '0_55',
+        '0_54',
+        '0_53',
+        '0_31',
+        '1_35',
+        '0_35',
+        '1_31',
+        '1_48',
+        '1_51',
+        '1_52',
+        '1_57',
+        '0_57',
+        '0_52',
+        '0_51',
+        '0_48',
+    ]
+
     @classmethod
     def can_translate(cls, header, filename=None):
         """Indicate whether this translation class can translate the
@@ -158,3 +272,25 @@ def to_detector_num(self):
     def to_detector_exposure_id(self):
         # Docstring will be inherited. Property defined in properties.py
         return self.to_exposure_id() * 200 + self.to_detector_num()
+
+    @cache_translation
+    def to_detector_group(self):
+        # Docstring will be inherited. Property defined in properties.py
+        unique = self.to_detector_unique_name()
+        return unique.split("_")[0]
+
+    @cache_translation
+    def to_detector_unique_name(self):
+        # Docstring will be inherited. Property defined in properties.py
+        # Mapping from number to unique name is defined solely in camera
+        # geom files.
+        # There is no header for it.
+        num = self.to_detector_num()
+        return self._DETECTOR_NUM_TO_UNIQUE_NAME[num]
+
+    @cache_translation
+    def to_detector_name(self):
+        # Docstring will be inherited. Property defined in properties.py
+        # Name is defined from unique name
+        unique = self.to_detector_unique_name()
+        return unique.split("_")[1]