Fix broken telescope id string, Fixes #1058 (#1069)

* Fixed indexing problems with Instrument Description classes

- get_tel_ids_for_type() was broken due to change in string representation, it is now fixed.
- added many unit tests to prevent regression

* update telescope naming in muon code

* update example

* updated tutorials

* fix one more telescope string

* fixed example and tutorial

* style cleanups

* use UNKNOWN_TELESCOPE on failed guess

* update test

* added some docstrings and reformatted

* pep8 style cleanups

* renamed type to tel_type to avoid name conflict with builtin

* pep8 style cleanup

* rename type to tel_type to avoid conflict with builtin

* add more docstrings

.coverage.dapmcw186.24220.071001

@@ -0,0 +1 @@
+!coverage.py: This is a private format, don't read it directly!{"lines":{}}

ctapipe/image/muon/muon_reco_functions.py

@@ -35,8 +35,8 @@ def analyze_muon_event(event):
 
     """
 
-    names = ['LST:LSTCam', 'MST:NectarCam', 'MST:FlashCam', 'MST-SCT:SCTCam',
-             '1M:DigiCam', 'GCT:CHEC', 'ASTRI:ASTRICam', 'ASTRI:CHEC']
+    names = ['LST_LST_LSTCam', 'MST_MST_NectarCam', 'MST_MST_FlashCam', 'MST_SCT_SCTCam',
+             'SST_1M_DigiCam', 'SST_GCT_CHEC', 'SST_ASTRI_ASTRICam', 'SST_ASTRI_CHEC']
     tail_cuts = [(5, 7), (5, 7), (10, 12), (5, 7),
                 (5, 7), (5, 7), (5, 7), (5, 7)]  # 10, 12?
     impact = [(0.2, 0.9), (0.1, 0.95), (0.2, 0.9), (0.2, 0.9),

ctapipe/instrument/guess.py

@@ -1,34 +1,39 @@
+"""
+Methods for guessing details about a telescope from some metadata (so that
+we can create TelescopeDescriptions from Monte-Carlo where some
+parameters like the names of the camera and optics structure are not
+stored in the file.
+"""
 from collections import namedtuple
-import astropy.units as u
 
+import astropy.units as u
 
-GuessingKey = namedtuple('GuessingKey', ['n_pixels', 'focal_length'])
+GuessingKey = namedtuple("GuessingKey", ["n_pixels", "focal_length"])
 GuessingResult = namedtuple(
-    'GuessingResult', ['type', 'name', 'camera_name', 'n_mirrors']
+    "GuessingResult", ["type", "name", "camera_name", "n_mirrors"]
 )
 
 
 TELESCOPE_NAMES = {
-    GuessingKey(2048, 2.28): GuessingResult('SST', 'GCT', 'CHEC', 2),
-    GuessingKey(2368, 2.15): GuessingResult('SST', 'ASTRI', 'ASTRICam', 2),
-    GuessingKey(2048, 2.15): GuessingResult('SST', 'ASTRI', 'CHEC', 2),
-    GuessingKey(1296, 5.60): GuessingResult('SST', '1M', 'DigiCam', 1),
-    GuessingKey(1764, 16.0): GuessingResult('MST', 'MST', 'FlashCam', 1),
-    GuessingKey(1855, 16.0): GuessingResult('MST', 'MST', 'NectarCam', 1),
-    GuessingKey(1855, 28.0): GuessingResult('LST', 'LST', 'LSTCam', 1),
-    GuessingKey(11328, 5.59): GuessingResult('MST', 'SCT', 'SCTCam', 1),
-
-    # None CTA Telescopes
-    GuessingKey(960, 15.0): GuessingResult('MST', 'HESS-I', 'HESS-I', 1),
-    GuessingKey(2048, 36.0): GuessingResult('LST', 'HESS-II', 'HESS-II', 1),
-    GuessingKey(1440, 4.998): GuessingResult('SST', 'FACT', 'FACT', 1),
+    GuessingKey(2048, 2.28): GuessingResult("SST", "GCT", "CHEC", 2),
+    GuessingKey(2368, 2.15): GuessingResult("SST", "ASTRI", "ASTRICam", 2),
+    GuessingKey(2048, 2.15): GuessingResult("SST", "ASTRI", "CHEC", 2),
+    GuessingKey(1296, 5.60): GuessingResult("SST", "1M", "DigiCam", 1),
+    GuessingKey(1764, 16.0): GuessingResult("MST", "MST", "FlashCam", 1),
+    GuessingKey(1855, 16.0): GuessingResult("MST", "MST", "NectarCam", 1),
+    GuessingKey(1855, 28.0): GuessingResult("LST", "LST", "LSTCam", 1),
+    GuessingKey(11328, 5.59): GuessingResult("MST", "SCT", "SCTCam", 1),
+    # Non-CTA Telescopes
+    GuessingKey(960, 15.0): GuessingResult("MST", "HESS-I", "HESS-I", 1),
+    GuessingKey(2048, 36.0): GuessingResult("LST", "HESS-II", "HESS-II", 1),
+    GuessingKey(1440, 4.998): GuessingResult("SST", "FACT", "FACT", 1),
 }
 
-UNKNOWN_TELESCOPE = GuessingResult('UNKNOWN', 'UNKNOWN', 'UNKNOWN', -1)
+UNKNOWN_TELESCOPE = GuessingResult("UNKNOWN", "UNKNOWN", "UNKNOWN", -1)
 
 
 def guess_telescope(n_pixels, focal_length):
-    '''
+    """
     From n_pixels of the camera and the focal_length,
     guess which telescope we are dealing with.
     This is mainly needed to add human readable names
@@ -45,12 +50,12 @@ def guess_telescope(n_pixels, focal_length):
     -------
     result: GuessingResult
         A namedtuple having type, telescope_name, camera_name and n_mirrors fields
-    '''
+    """
 
     # allow unit input
     focal_length = u.Quantity(focal_length, u.m).to_value(u.m)
 
     try:
         return TELESCOPE_NAMES[(n_pixels, round(focal_length, 2))]
     except KeyError:
-        raise ValueError(f'Unknown telescope: n_pixel={n_pixels}, f={focal_length}')
+        raise ValueError(f"Unknown telescope: n_pixel={n_pixels}, f={focal_length}")

ctapipe/instrument/optics.py

@@ -3,9 +3,11 @@
 """
 
 import logging
-from ..utils import get_table_dataset
-import numpy as np
+
 import astropy.units as u
+import numpy as np
+
+from ..utils import get_table_dataset
 
 logger = logging.getLogger(__name__)
 
@@ -41,14 +43,14 @@ class OpticsDescription:
         if the units of one of the inputs are missing or incompatible
     """
 
-    @u.quantity_input(mirror_area=u.m**2, equivalent_focal_length=u.m)
+    @u.quantity_input(mirror_area=u.m ** 2, equivalent_focal_length=u.m)
     def __init__(
-            self,
-            name,
-            num_mirrors,
-            equivalent_focal_length,
-            mirror_area=None,
-            num_mirror_tiles=None
+        self,
+        name,
+        num_mirrors,
+        equivalent_focal_length,
+        mirror_area=None,
+        num_mirror_tiles=None,
     ):
 
         self.name = name
@@ -58,20 +60,22 @@ def __init__(
         self.num_mirror_tiles = num_mirror_tiles
 
     def __hash__(self):
-        '''Make this hashable, so it can be used as dict keys or in sets'''
-        return hash((
-            self.equivalent_focal_length.to_value(u.m),
-            self.mirror_area,
-            self.num_mirrors,
-            self.num_mirror_tiles,
-        ))
+        """Make this hashable, so it can be used as dict keys or in sets"""
+        return hash(
+            (
+                self.equivalent_focal_length.to_value(u.m),
+                self.mirror_area,
+                self.num_mirrors,
+                self.num_mirror_tiles,
+            )
+        )
 
     def __eq__(self, other):
-        '''Make this hashable, so it can be used as dict keys or in sets'''
+        """Make this hashable, so it can be used as dict keys or in sets"""
         return hash(self) == hash(other)
 
     @classmethod
-    def from_name(cls, name, optics_table='optics'):
+    def from_name(cls, name, optics_table="optics"):
         """
         Construct an OpticsDescription from the name. This is loaded from
         `optics.fits.gz`, which should be in `ctapipe_resources` or in a
@@ -90,27 +94,32 @@ def from_name(cls, name, optics_table='optics'):
         OpticsDescription
 
         """
-        table = get_table_dataset(optics_table, role='dl0.tel.svc.optics')
-        mask = table['tel_description'] == name
+        table = get_table_dataset(optics_table, role="dl0.tel.svc.optics")
+        mask = table["tel_description"] == name
         if mask.sum() == 0:
-            raise ValueError(f'Unknown telescope name {name}')
+            raise ValueError(f"Unknown telescope name {name}")
 
-        flen = table['equivalent_focal_length'][mask].quantity[0]
+        flen = table["equivalent_focal_length"][mask].quantity[0]
 
-        num_mirrors = 1 if table['mirror_type'][mask][0] == 'DC' else 2
+        num_mirrors = 1 if table["mirror_type"][mask][0] == "DC" else 2
         optics = cls(
             name=name,
             num_mirrors=num_mirrors,
             equivalent_focal_length=flen,
-            mirror_area=table['mirror_area'][mask].quantity[0],
-            num_mirror_tiles=table['num_mirror_tiles'][mask][0],
+            mirror_area=table["mirror_area"][mask].quantity[0],
+            num_mirror_tiles=table["num_mirror_tiles"][mask][0],
         )
         return optics
 
     @classmethod
-    def get_known_optics_names(cls, optics_table='optics'):
-        table = get_table_dataset(optics_table, 'get_known_optics')
-        return np.array(table['tel_description'])
+    def get_known_optics_names(cls, optics_table="optics"):
+        """
+        return the list of optics names from ctapipe resources, i.e. those that can be
+        constructed by name (this does not return the list of known names from an
+        already open Monte-Carlo file)
+        """
+        table = get_table_dataset(optics_table, "get_known_optics")
+        return np.array(table["tel_description"])
 
     def __repr__(self):
         return (