Repository: cta-observatory/ctapipe
Pull request #1932 (2022-06-14 08:59:16)

Add overwrite option to write_table
`write_table` always overwrote an existing table for `append=False`.

To bring this inline with our other IO methods, which by default don't perform potentially destructive actions, I add the overwrite option here which is by default False.


Patch:
diff --git a/ctapipe/instrument/subarray.py b/ctapipe/instrument/subarray.py
index 24ac847b..4ac5fd77 100644
--- a/ctapipe/instrument/subarray.py
+++ b/ctapipe/instrument/subarray.py
@@ -461,43 +461,35 @@ class SubarrayDescription:
             if not isinstance(h5file, tables.File):
                 h5file = stack.enter_context(tables.open_file(h5file, mode=mode))
 
-            if "/configuration/instrument/subarray" in h5file.root:
-                if overwrite is False:
-                    raise IOError(
-                        "File already contains a SubarrayDescription and overwrite=False"
-                    )
-
-                h5file.remove_node(
-                    "/configuration/instrument/", "subarray", recursive=True
-                )
-                h5file.remove_node(
-                    "/configuration/instrument/", "telescope", recursive=True
+            if "/configuration/instrument/subarray" in h5file.root and not overwrite:
+                raise IOError(
+                    "File already contains a SubarrayDescription and overwrite=False"
                 )
 
             write_table(
                 self.to_table(),
                 h5file,
                 path="/configuration/instrument/subarray/layout",
-                mode="a",
+                overwrite=overwrite,
             )
             write_table(
                 self.to_table(kind="optics"),
                 h5file,
                 path="/configuration/instrument/telescope/optics",
-                mode="a",
+                overwrite=overwrite,
             )
             for i, camera in enumerate(self.camera_types):
                 write_table(
                     camera.geometry.to_table(),
                     h5file,
                     path=f"/configuration/instrument/telescope/camera/geometry_{i}",
-                    mode="a",
+                    overwrite=overwrite,
                 )
                 write_table(
                     camera.readout.to_table(),
                     h5file,
                     path=f"/configuration/instrument/telescope/camera/readout_{i}",
-                    mode="a",
+                    overwrite=overwrite,
                 )
 
             h5file.root.configuration.instrument.subarray._v_attrs.name = self.name
diff --git a/ctapipe/io/astropy_helpers.py b/ctapipe/io/astropy_helpers.py
index cc12d413..aca84323 100644
--- a/ctapipe/io/astropy_helpers.py
+++ b/ctapipe/io/astropy_helpers.py
@@ -95,7 +95,15 @@ def read_table(
         return astropy_table
 
 
-def write_table(table, h5file, path, append=False, mode="a", filters=DEFAULT_FILTERS):
+def write_table(
+    table,
+    h5file,
+    path,
+    append=False,
+    overwrite=False,
+    mode="a",
+    filters=DEFAULT_FILTERS,
+):
     """Write a table to an HDF5 file
 
     This writes a table in the ctapipe format into ``h5file``.
@@ -113,16 +121,34 @@ def write_table(table, h5file, path, append=False, mode="a", filters=DEFAULT_FIL
         dataset path inside the ``h5file``
     append: bool
         Wether to try to append to or replace an existing table
+    overwrite: bool
+        If table is already in file and overwrite and append are false,
+        raise an error.
     mode: str
         If given a path for ``h5file``, it will be opened in this mode.
         See the docs of ``tables.open_file``.
     """
     copied = False
+    parent, table_name = os.path.split(path)
+
+    if append and overwrite:
+        raise ValueError("overwrite and append are mutually exclusive")
 
     with ExitStack() as stack:
         if not isinstance(h5file, tables.File):
             h5file = stack.enter_context(tables.open_file(h5file, mode=mode))
 
+        already_exists = path in h5file.root
+        if already_exists:
+            if overwrite and not append:
+                h5file.remove_node(parent, table_name)
+                already_exists = False
+
+            elif not overwrite and not append:
+                raise IOError(
+                    f"Table {path} already exists in output file, use append or overwrite"
+                )
+
         attrs = {}
         for colname, column in table.columns.items():
             if hasattr(column, "description") and column.description is not None:
@@ -153,14 +179,6 @@ def write_table(table, h5file, path, append=False, mode="a", filters=DEFAULT_FIL
                 transform = QuantityColumnTransform(column.unit)
                 attrs.update(transform.get_meta(colname))
 
-        parent, table_name = os.path.split(path)
-
-        already_exists = path in h5file.root
-
-        if already_exists and not append:
-            h5file.remove_node(parent, table_name)
-            already_exists = False
-
         if not already_exists:
             h5_table = h5file.create_table(
                 parent,


Test patch:
diff --git a/ctapipe/io/tests/test_write_table.py b/ctapipe/io/tests/test_write_table.py
index dacdf7e5..54146475 100644
--- a/ctapipe/io/tests/test_write_table.py
+++ b/ctapipe/io/tests/test_write_table.py
@@ -45,13 +45,17 @@ def test_write_table(tmp_path):
     assert read.meta["FOO"] == "bar"
     assert read["speed"].description == "Speed of stuff"
 
+    # test error for already existing table
+    with pytest.raises(IOError):
+        write_table(table, output_path, table_path)
+
     # test we can append
     write_table(table, output_path, table_path, append=True)
     read = read_table(output_path, table_path)
     assert len(read) == 2 * len(table)
 
     # test we can overwrite
-    write_table(table, output_path, table_path, append=False)
+    write_table(table, output_path, table_path, overwrite=True)
     assert len(read_table(output_path, table_path)) == len(table)