Repository: cta-observatory/ctapipe
Pull request #1992 (2022-07-04 11:51:49)

Ignore unknown column metadata in read_table
This does not fully address #1785, but it makes the resulting tables at least readable using `read_table`.


Patch:
diff --git a/ctapipe/core/container.py b/ctapipe/core/container.py
index faa6b325..68322fc7 100644
--- a/ctapipe/core/container.py
+++ b/ctapipe/core/container.py
@@ -135,20 +135,6 @@ class Field:
                 f"{errorstr} Should be an instance of {self.type}"
             )
 
-        if isinstance(value, Container):
-            # recursively check sub-containers
-            value.validate()
-            return
-
-        if isinstance(value, Map):
-            for key, map_value in value.items():
-                if isinstance(map_value, Container):
-                    try:
-                        map_value.validate()
-                    except FieldValidationError as err:
-                        raise FieldValidationError(f"[{key}]: {err} ")
-            return
-
         if self.unit is not None:
             if not isinstance(value, Quantity):
                 raise FieldValidationError(
diff --git a/ctapipe/io/astropy_helpers.py b/ctapipe/io/astropy_helpers.py
index bcd2687c..5e75e4fc 100644
--- a/ctapipe/io/astropy_helpers.py
+++ b/ctapipe/io/astropy_helpers.py
@@ -89,9 +89,15 @@ def read_table(
 
         astropy_table = table_cls(array, meta=meta, copy=False)
         for column, tr in transforms.items():
+            if column not in astropy_table.colnames:
+                continue
+
             astropy_table[column] = tr.inverse(astropy_table[column])
 
         for column, desc in descriptions.items():
+            if column not in astropy_table.colnames:
+                continue
+
             astropy_table[column].description = desc
 
         return astropy_table
diff --git a/examples/load_one_event.py b/examples/load_one_event.py
index cf17b384..facf5cca 100644
--- a/examples/load_one_event.py
+++ b/examples/load_one_event.py
@@ -15,11 +15,9 @@ if __name__ == "__main__":
     if len(sys.argv) >= 2:
         filename = sys.argv[1]
     else:
-        filename = get_dataset_path(
-            "gamma_LaPalma_baseline_20Zd_180Az_prod3b_test.simtel.gz"
-        )
+        filename = get_dataset_path("gamma_test_large.simtel.gz")
 
-    with EventSource(filename, max_events=1, focal_length_choice="nominal") as source:
+    with EventSource(filename, max_events=1) as source:
         calib = CameraCalibrator(subarray=source.subarray)
         process_images = ImageProcessor(subarray=source.subarray)
         process_shower = ShowerProcessor(subarray=source.subarray)


Test patch:
diff --git a/ctapipe/core/tests/test_container.py b/ctapipe/core/tests/test_container.py
index 1dc1409e..0c0e0a01 100644
--- a/ctapipe/core/tests/test_container.py
+++ b/ctapipe/core/tests/test_container.py
@@ -255,7 +255,7 @@ def test_field_validation():
 
 
 def test_container_validation():
-    """check that we can validate all fields in a container"""
+    """ check that we can validate all fields in a container"""
 
     class MyContainer(Container):
         x = Field(3.2, "test", unit="m")
@@ -264,29 +264,4 @@ def test_container_validation():
     with pytest.raises(FieldValidationError):
         MyContainer().validate()  # fails since 3.2 has no units
 
-    with pytest.raises(FieldValidationError):
-        MyContainer(x=10 * u.s).validate()  # seconds is not convertable to meters
-
     MyContainer(x=6.4 * u.m).validate()  # works
-
-
-def test_recursive_validation():
-    """
-    Check both sub-containers and Maps work with recursive validation
-    """
-
-    class ChildContainer(Container):
-        x = Field(3.2 * u.m, "test", unit="m")
-
-    class ParentContainer(Container):
-        cont = Field(None, "test sub", type=ChildContainer)
-        map = Field(Map(ChildContainer), "many children")
-
-    with pytest.raises(FieldValidationError):
-        ParentContainer(cont=ChildContainer(x=1 * u.s)).validate()
-
-    with pytest.raises(FieldValidationError):
-        cont = ParentContainer(cont=ChildContainer(x=1 * u.m))
-        cont.map[0] = ChildContainer(x=1 * u.m)
-        cont.map[1] = ChildContainer(x=1 * u.s)
-        cont.validate()
diff --git a/ctapipe/io/tests/test_astropy_helpers.py b/ctapipe/io/tests/test_astropy_helpers.py
index a7504b3c..718caf09 100644
--- a/ctapipe/io/tests/test_astropy_helpers.py
+++ b/ctapipe/io/tests/test_astropy_helpers.py
@@ -188,3 +188,22 @@ def test_read_table_astropy(tmp_path):
     table.write(path, "/group/table", serialize_meta=True)
     read = read_table(path, "/group/table")
     assert_table_equal(table, read)
+
+
+def test_unknown_meta(tmp_path):
+    """Test read_table ignores unknown metadata
+
+    Regression test for https://github.com/cta-observatory/ctapipe/issues/1785
+    """
+    container = ReconstructedEnergyContainer()
+    filename = tmp_path / "test_astropy_table.h5"
+
+    with HDF5TableWriter(filename) as writer:
+        for energy in [np.nan, 100, np.nan, 50, -1.0] * u.TeV:
+            container.energy = energy
+            writer.write("events", container)
+
+        # unknown column matching ctapipe metadata
+        writer.h5file.root.events._v_attrs["foo_UNIT"] = "TeV"
+
+    read_table(filename, "/events", condition="energy > 0")