Merge pull request #1925 from cta-observatory/select_subarray

Better error message in case unknown telescopes are selected, fixes #1924

ctapipe/instrument/__init__.py

@@ -2,7 +2,7 @@
 from .atmosphere import get_atmosphere_profile_functions
 from .telescope import TelescopeDescription
 from .optics import OpticsDescription
-from .subarray import SubarrayDescription
+from .subarray import SubarrayDescription, UnknownTelescopeID
 from .guess import guess_telescope
 
 
@@ -11,9 +11,10 @@
     "CameraGeometry",
     "CameraReadout",
     "get_atmosphere_profile_functions",
-    "TelescopeDescription",
-    "OpticsDescription",
-    "SubarrayDescription",
     "guess_telescope",
+    "OpticsDescription",
     "PixelShape",
+    "SubarrayDescription",
+    "TelescopeDescription",
+    "UnknownTelescopeID",
 ]

ctapipe/instrument/subarray.py

@@ -23,12 +23,17 @@
 __all__ = ["SubarrayDescription"]
 
 
+class UnknownTelescopeID(KeyError):
+    """Raised when an unknown telescope id is encountered"""
+
+
 def _group_consecutives(sequence):
     """
     Turn consequtive lists into ranges (used in SubarrayDescription.info())
 
     from https://codereview.stackexchange.com/questions/214820/codewars-range-extraction
     """
+    sequence = sorted(sequence)
     for _, g in groupby(enumerate(sequence), lambda i_x: i_x[0] - i_x[1]):
         r = [x for _, x in g]
         if len(r) > 2:
@@ -314,11 +319,15 @@ def select_subarray(self, tel_ids, name=None):
         SubarrayDescription
         """
 
+        unknown_tel_ids = set(tel_ids).difference(self.tel.keys())
+        if len(unknown_tel_ids) > 0:
+            known = _range_extraction(self.tel.keys())
+            raise UnknownTelescopeID(f"{unknown_tel_ids}, known telescopes: {known}")
+
         tel_positions = {tid: self.positions[tid] for tid in tel_ids}
         tel_descriptions = {tid: self.tel[tid] for tid in tel_ids}
 
         if not name:
-            tel_ids = sorted(tel_ids)
             name = self.name + "_" + _range_extraction(tel_ids)
 
         newsub = SubarrayDescription(

ctapipe/instrument/tests/test_subarray.py

@@ -16,7 +16,7 @@
 
 
 def example_subarray(n_tels=10):
-    """ generate a simple subarray for testing purposes """
+    """generate a simple subarray for testing purposes"""
     rng = np.random.default_rng(0)
 
     pos = {}
@@ -32,7 +32,7 @@ def example_subarray(n_tels=10):
 
 
 def test_subarray_description():
-    """ Test SubarrayDescription functionality """
+    """Test SubarrayDescription functionality"""
     n_tels = 10
     sub = example_subarray(n_tels)
     sub.peek()
@@ -67,14 +67,14 @@ def test_subarray_description():
 
 
 def test_to_table(example_subarray):
-    """ Check that we can generate astropy Tables from the SubarrayDescription """
+    """Check that we can generate astropy Tables from the SubarrayDescription"""
     sub = example_subarray
     assert len(sub.to_table(kind="subarray")) == sub.num_tels
     assert len(sub.to_table(kind="optics")) == len(sub.optics_types)
 
 
 def test_tel_indexing(example_subarray):
-    """ Check that we can convert between telescope_id and telescope_index """
+    """Check that we can convert between telescope_id and telescope_index"""
     sub = example_subarray
 
     assert sub.tel_indices[1] == 0  # first tel_id is in slot 0
@@ -218,3 +218,10 @@ def test_get_tel_ids(example_subarray):
     # test invalid telescope type
     with pytest.raises(Exception):
         tel_ids = subarray.get_tel_ids(["It's a-me, Mario!"])
+
+
+def test_unknown_telescopes(example_subarray):
+    from ctapipe.instrument import UnknownTelescopeID
+
+    with pytest.raises(UnknownTelescopeID):
+        example_subarray.select_subarray([300, 201])