Repository: cta-observatory/ctapipe
Pull request #2004 (2022-07-12 12:30:45)

Fix datalevels


Patch:
diff --git a/ctapipe/instrument/guess.py b/ctapipe/instrument/guess.py
index 838a72e4..f2d04653 100644
--- a/ctapipe/instrument/guess.py
+++ b/ctapipe/instrument/guess.py
@@ -1,5 +1,5 @@
 """
-Methods for guessing details about a telescope from some metadata, so that
+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.
@@ -9,63 +9,32 @@ from collections import namedtuple
 import astropy.units as u
 import numpy as np
 
-
-GuessingKey = namedtuple(
-    "GuessingKey",
-    ["n_pixels", "focal_length", "num_mirror_tiles"],
-    defaults=(None,),  # Mirror Tiles are optional
-)
-
+GuessingKey = namedtuple("GuessingKey", ["n_pixels", "focal_length"])
 GuessingResult = namedtuple(
     "GuessingResult", ["type", "name", "camera_name", "n_mirrors"]
 )
 
 
-def _build_lookup_tree(telescopes):
-    tree = {}
-
-    for k, v in telescopes:
-        if k.n_pixels not in tree:
-            tree[k.n_pixels] = {}
-
-        if k.focal_length not in tree[k.n_pixels]:
-            tree[k.n_pixels][k.focal_length] = {}
-
-        d = tree[k.n_pixels][k.focal_length]
-
-        if k.num_mirror_tiles in d:
-            other = d[k.num_mirror_tiles]
-            raise ValueError(f"GuessingKeys are not unique: {k}: {v}, {other}")
-
-        d[k.num_mirror_tiles] = v
-
-    return tree
-
-
 # focal length must have at most two digits after period
 # as we round the lookup to two digits
-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)),
+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),
     # Non-CTA Telescopes
-    (GuessingKey(1039, 16.97, 964), GuessingResult("LST", "MAGIC-1", "MAGICCam", 1)),
-    (GuessingKey(1039, 16.97, 247), GuessingResult("LST", "MAGIC-2", "MAGICCam", 1)),
-    (GuessingKey(1039, 17.0, 964), GuessingResult("LST", "MAGIC-1", "MAGICCam", 1)),
-    (GuessingKey(1039, 17.0, 247), GuessingResult("LST", "MAGIC-2", "MAGICCam", 1)),
-    (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.89), GuessingResult("SST", "FACT", "FACT", 1)),
-]
-LOOKUP_TREE = _build_lookup_tree(TELESCOPE_NAMES)
+    GuessingKey(1039, 16.97): GuessingResult("LST", "MAGIC", "MAGICCam", 1),
+    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.89): GuessingResult("SST", "FACT", "FACT", 1),
+}
 
 
-def guess_telescope(n_pixels, focal_length, num_mirror_tiles=None):
+def guess_telescope(n_pixels, focal_length):
     """
     From n_pixels of the camera and the focal_length,
     guess which telescope we are dealing with.
@@ -84,24 +53,17 @@ def guess_telescope(n_pixels, focal_length, num_mirror_tiles=None):
     result: GuessingResult
         A namedtuple having type, telescope_name, camera_name and n_mirrors fields
     """
-    focal_length = round(u.Quantity(focal_length, u.m).to_value(u.m), 2)
+    focal_length = u.Quantity(focal_length, u.m).to_value(u.m)
 
     try:
-        d = LOOKUP_TREE[n_pixels][focal_length]
-        # first check with num_mirror_tiles
-        tel = d.get(num_mirror_tiles)
-        if tel is not None:
-            return tel
-
-        # fallback to "not needed mirror tiles"
-        return d[None]
+        return TELESCOPE_NAMES[(n_pixels, round(focal_length, 2))]
     except KeyError:
         raise ValueError(f"Unknown telescope: n_pixel={n_pixels}, f={focal_length}")
 
 
 def unknown_telescope(mirror_area, n_pixels, n_mirrors=-1):
     """Create a GuessingResult for an unknown_telescope"""
-    mirror_area = u.Quantity(mirror_area, u.m**2).to_value(u.m**2)
+    mirror_area = u.Quantity(mirror_area, u.m ** 2).to_value(u.m ** 2)
 
     mirror_diameter = 2 * np.sqrt(mirror_area / np.pi)
     if mirror_diameter < 8:
diff --git a/ctapipe/io/datawriter.py b/ctapipe/io/datawriter.py
index 6fea8fb1..36ef959e 100644
--- a/ctapipe/io/datawriter.py
+++ b/ctapipe/io/datawriter.py
@@ -104,7 +104,7 @@ def write_reference_metadata_headers(
         product=meta.Product(
             description="ctapipe Data Product",
             data_category=category,
-            data_level=[level.name for level in data_levels],
+            data_levels=data_levels,
             data_association="Subarray",
             data_model_name="ASWG",
             data_model_version=DATA_MODEL_VERSION,
diff --git a/ctapipe/io/hdf5eventsource.py b/ctapipe/io/hdf5eventsource.py
index 29160ac9..fa1d1bef 100644
--- a/ctapipe/io/hdf5eventsource.py
+++ b/ctapipe/io/hdf5eventsource.py
@@ -1,5 +1,4 @@
 import logging
-from ast import literal_eval
 
 import astropy.units as u
 import numpy as np
@@ -193,11 +192,11 @@ class HDF5EventSource(EventSource):
 
         with tables.open_file(file_path) as f:
             metadata = f.root._v_attrs
-            if "CTA PRODUCT DATA LEVEL" not in metadata._v_attrnames:
+            if "CTA PRODUCT DATA LEVELS" not in metadata._v_attrnames:
                 return False
 
             # we can now read both R1 and DL1
-            datalevels = set(literal_eval(metadata["CTA PRODUCT DATA LEVEL"]))
+            datalevels = set(metadata["CTA PRODUCT DATA LEVELS"].split(","))
             if not datalevels.intersection(("R1", "DL1_IMAGES", "DL1_PARAMETERS")):
                 return False
 
diff --git a/ctapipe/io/metadata.py b/ctapipe/io/metadata.py
index da77c6cd..e61999a6 100644
--- a/ctapipe/io/metadata.py
+++ b/ctapipe/io/metadata.py
@@ -22,19 +22,19 @@ them (as in `Activity.from_provenance()`)
     some_astropy_table.write("output.ecsv")
 
 """
+import os
+import pwd
 import uuid
 import warnings
 from collections import OrderedDict
-import os
-import pwd
+from contextlib import ExitStack
+from pathlib import Path
 
 import tables
 from astropy.time import Time
 from tables import NaturalNameWarning
-from traitlets import Enum, Instance, List, Unicode, default, HasTraits
+from traitlets import Enum, HasTraits, Instance, List, Unicode, UseEnum, default
 from traitlets.config import Configurable
-from contextlib import ExitStack
-from pathlib import Path
 
 from ..core.traits import AstroTime
 from .datalevels import DataLevel
@@ -51,7 +51,18 @@ __all__ = [
 ]
 
 
-CONVERSIONS = {Time: lambda t: t.utc.iso, list: str}
+CONVERSIONS = {
+    Time: lambda t: t.utc.iso,
+    list: lambda l: ",".join([convert(elem) for elem in l]),
+    DataLevel: lambda d: d.name,
+}
+
+
+def convert(value):
+    """Convert to representation suitable for header infos, such as hdf5 or fits"""
+    if (conv := CONVERSIONS.get(type(value))) is not None:
+        return conv(value)
+    return value
 
 
 class Contact(Configurable):
@@ -80,7 +91,7 @@ class Product(HasTraits):
     creation_time = AstroTime()
     id_ = Unicode(help="leave unspecified to automatically generate a UUID")
     data_category = Enum(["Sim", "A", "B", "C", "Other"], "Other")
-    data_level = List(Enum([level.name for level in DataLevel]))
+    data_levels = List(UseEnum(DataLevel))
     data_association = Enum(["Subarray", "Telescope", "Target", "Other"], "Other")
     data_model_name = Unicode("unknown")
     data_model_version = Unicode("unknown")
@@ -198,7 +209,7 @@ def _to_dict(hastraits_instance, prefix=""):
         val = trait.get(hastraits_instance)
 
         # apply type conversions
-        val = CONVERSIONS.get(type(val), lambda v: v)(val)
+        val = convert(val)
         res[key] = val
 
     return res
@@ -280,4 +291,4 @@ def read_metadata(h5file, path="/"):
 
         node = h5file.get_node(path)
         metadata = {key: node._v_attrs[key] for key in node._v_attrs._f_list()}
-    return metadata
+        return metadata
diff --git a/ctapipe/io/simteleventsource.py b/ctapipe/io/simteleventsource.py
index e19abf1a..a4cd6917 100644
--- a/ctapipe/io/simteleventsource.py
+++ b/ctapipe/io/simteleventsource.py
@@ -318,11 +318,7 @@ class SimTelEventSource(EventSource):
             )
 
             try:
-                telescope = guess_telescope(
-                    n_pixels,
-                    equivalent_focal_length,
-                    cam_settings["n_mirrors"],
-                )
+                telescope = guess_telescope(n_pixels, equivalent_focal_length)
             except ValueError:
                 telescope = unknown_telescope(mirror_area, n_pixels)
 
diff --git a/ctapipe/tools/merge.py b/ctapipe/tools/merge.py
index e965d14d..27546b84 100644
--- a/ctapipe/tools/merge.py
+++ b/ctapipe/tools/merge.py
@@ -1,24 +1,23 @@
 """
 Merge DL1-files from ctapipe-process tool
 """
-import sys
 import os
+import sys
 from argparse import ArgumentParser
 from pathlib import Path
-from traitlets import List
 
-import tables
 import numpy as np
+import tables
 from tqdm.auto import tqdm
+from traitlets import List
 
 from ctapipe.utils.arrays import recarray_drop_columns
 
-from ..io import metadata as meta, HDF5EventSource, get_hdf5_datalevels
-from ..io import HDF5TableWriter
 from ..core import Provenance, Tool, traits
-from ..core.traits import Bool, Set, Unicode, flag, CInt
+from ..core.traits import Bool, CInt, Set, Unicode, flag
 from ..instrument import SubarrayDescription
-
+from ..io import HDF5EventSource, HDF5TableWriter, get_hdf5_datalevels
+from ..io import metadata as meta
 
 PROV = Provenance()
 
@@ -386,7 +385,9 @@ class MergeTool(Tool):
                 input_table = recarray_drop_columns(input_node[:], filter_columns)
                 where, name = os.path.split(node_path)
                 self.output_file.create_table(
-                    where, name, filters=input_node.filters,
+                    where,
+                    name,
+                    filters=input_node.filters,
                     createparents=True,
                     obj=input_table,
                 )
@@ -474,7 +475,7 @@ class MergeTool(Tool):
             product=meta.Product(
                 description="Merged DL1 Data Product",
                 data_category="Sim",  # TODO: copy this from the inputs
-                data_level=datalevels,
+                data_levels=datalevels,
                 data_association="Subarray",
                 data_model_name="ASWG",  # TODO: copy this from inputs
                 data_model_version=self.data_model_version,


Test patch:
diff --git a/ctapipe/instrument/tests/test_guessing.py b/ctapipe/instrument/tests/test_guessing.py
index d1c66d74..b71bc970 100644
--- a/ctapipe/instrument/tests/test_guessing.py
+++ b/ctapipe/instrument/tests/test_guessing.py
@@ -5,12 +5,11 @@ from pytest import raises
 def test_guessing():
     from ctapipe.instrument import guess_telescope
 
-    # n_tiles should not be used for GCT since pixels + focal length is unique
-    guess = guess_telescope(2048, 2.28, None)
+    guess = guess_telescope(2048, 2.28)
     assert guess.type == "SST"
     assert guess.name == "GCT"
 
-    guess = guess_telescope(2048, 2.28 * u.m, 32)
+    guess = guess_telescope(2048, 2.28 * u.m)
     assert guess.type == "SST"
     assert guess.name == "GCT"
 
@@ -24,9 +23,6 @@ def test_guessing():
     assert guess.camera_name == "FlashCam"
     assert guess.type == "MST"
 
-    assert guess_telescope(1039, 16.97, 964).name == "MAGIC-1"
-    assert guess_telescope(1039, 16.97, 247).name == "MAGIC-2"
-
 
 def test_unknown_telescope():
     from ctapipe.instrument.guess import unknown_telescope
@@ -41,7 +37,7 @@ def test_unknown_telescope():
     assert tel.name == "UNKNOWN-10M2"
     assert tel.camera_name == "UNKNOWN-2048PX"
 
-    tel = unknown_telescope(100 * u.m**2, 2048)
+    tel = unknown_telescope(100 * u.m ** 2, 2048)
     assert tel.type == "MST"
     assert tel.name == "UNKNOWN-100M2"
     assert tel.camera_name == "UNKNOWN-2048PX"
diff --git a/ctapipe/io/tests/test_metadata.py b/ctapipe/io/tests/test_metadata.py
index 26f2c07e..af16200e 100644
--- a/ctapipe/io/tests/test_metadata.py
+++ b/ctapipe/io/tests/test_metadata.py
@@ -3,12 +3,13 @@ Test CTA Reference metadata functionality
 """
 
 import tables
-from ctapipe.io import metadata as meta
+
 from ctapipe.core.provenance import Provenance
+from ctapipe.io import metadata as meta
 
 
 def test_construct_and_write_metadata(tmp_path):
-    """ basic test of making a Reference object and writing it"""
+    """basic test of making a Reference object and writing it"""
 
     prov = Provenance()
     prov.start_activity("test")
@@ -23,7 +24,7 @@ def test_construct_and_write_metadata(tmp_path):
             description="An Amazing Product",
             creation_time="2020-10-11 15:23:31",
             data_category="Sim",
-            data_level=["DL1_IMAGES", "DL1_PARAMETERS"],
+            data_levels=["DL1_IMAGES", "DL1_PARAMETERS"],
             data_association="Subarray",
             data_model_name="Unofficial DL1",
             data_model_version="1.0",
@@ -43,6 +44,7 @@ def test_construct_and_write_metadata(tmp_path):
 
     ref_dict = reference.to_dict()
     assert ref_dict["CTA PRODUCT FORMAT"] == "hdf5"
+    assert ref_dict["CTA PRODUCT DATA LEVELS"] == "DL1_IMAGES,DL1_PARAMETERS"
 
     import uuid  # pylint: disable=import-outside-toplevel
 
@@ -69,26 +71,23 @@ def test_construct_and_write_metadata(tmp_path):
     import tables  # pylint: disable=import-outside-toplevel
 
     with tables.open_file(tmp_path / "test.h5", mode="w") as h5file:
-        h5file.create_group(where='/', name='node')
-        meta.write_to_hdf5(ref_dict, h5file, path='/node')
+        h5file.create_group(where="/", name="node")
+        meta.write_to_hdf5(ref_dict, h5file, path="/node")
 
 
 def test_read_metadata(tmp_path):
     # Testing one can read both a path as well as a PyTables file object
     filename = tmp_path / "test.h5"
-    metadata_in = {
-        'SOFTWARE': 'ctapipe',
-        'FOO': 'BAR'
-    }
-    metadata_path = '/node/subnode'
+    metadata_in = {"SOFTWARE": "ctapipe", "FOO": "BAR"}
+    metadata_path = "/node/subnode"
     with tables.open_file(filename, mode="w") as h5file:
-        h5file.create_group(where='/node', name='subnode', createparents=True)
+        h5file.create_group(where="/node", name="subnode", createparents=True)
         meta.write_to_hdf5(metadata_in, h5file, path=metadata_path)
 
     metadata_out = meta.read_metadata(filename, path=metadata_path)
     assert metadata_out == metadata_in
 
-    with tables.open_file(filename, 'r') as file:
+    with tables.open_file(filename, "r") as file:
         metadata_out = meta.read_metadata(file, path=metadata_path)
 
     assert metadata_out == metadata_in