From 452ee9ed36492daf17c002b1154745fd94462e78 Mon Sep 17 00:00:00 2001
From: Spencer Wong <spencer.wong@anu.edu.au>
Date: Sun, 18 Aug 2024 12:54:33 +1000
Subject: [PATCH 01/12] Add field file deletion to conversion driver

---
 umpost/conversion_driver_esm1p5.py | 31 ++++++++++++++++++++----------
 1 file changed, 21 insertions(+), 10 deletions(-)

diff --git a/umpost/conversion_driver_esm1p5.py b/umpost/conversion_driver_esm1p5.py
index 3fa2fc4..a4cdf99 100755
--- a/umpost/conversion_driver_esm1p5.py
+++ b/umpost/conversion_driver_esm1p5.py
@@ -126,7 +126,7 @@ def find_matching_fields_files(dir_contents, fields_file_name_pattern):
     return fields_file_paths
 
 
-def convert_fields_file_list(fields_file_paths, nc_write_dir):
+def convert_fields_file_list(fields_file_paths, nc_write_dir, delete_ff):
     """
     Convert group of fields files to NetCDF, writing output in nc_write_dir.
 
@@ -134,6 +134,8 @@ def convert_fields_file_list(fields_file_paths, nc_write_dir):
     ----------
     fields_file_paths : list of paths to fields files for conversion.
     nc_write_dir : directory to save NetCDF files into.
+    delete_ff : Boolean specifying whether to delete fields files upon
+                  successful conversion.
 
     Returns
     -------
@@ -154,6 +156,8 @@ def convert_fields_file_list(fields_file_paths, nc_write_dir):
         try:
             um2netcdf4.process(fields_file_path, nc_write_path, ARG_VALS)
             succeeded.append((fields_file_path, nc_write_path))
+            if delete_ff:
+                fields_file_path.unlink()
 
         except Exception as exc:
             # TODO: Refactor once um2nc has specific exceptions
@@ -171,7 +175,7 @@ def format_successes(succeeded):
     Format reports of successful conversions to be shared with user.
 
     Parameters
-    ---------- 
+    ----------
     succeeded: list of (input, output) tuples of filepaths for successful
                conversions.
 
@@ -179,7 +183,7 @@ def format_successes(succeeded):
     -------
     success_report: formatted report of successful conversion.
     """
-    
+
     for input_path, output_path in succeeded:
         success_report = f"Successfully converted {input_path} to {output_path}"
         yield success_report
@@ -191,9 +195,9 @@ def format_failures(failed, quiet):
 
     Parameters
     ----------
-    failed: list of tuples of form (filepath, exception) for files which failed 
+    failed: list of tuples of form (filepath, exception) for files which failed
             to convert due to an allowable exception.
-    quiet: boolean. Report only final exception type and message rather than 
+    quiet: boolean. Report only final exception type and message rather than
            full stack trace when true.
 
     Yields
@@ -222,14 +226,16 @@ def format_failures(failed, quiet):
             yield failure_report
 
 
-def convert_esm1p5_output_dir(esm1p5_output_dir):
+def convert_esm1p5_output_dir(esm1p5_output_dir, delete_ff):
     """
     Driver function for converting ESM1.5 atmospheric outputs during a simulation.
 
     Parameters
     ----------
-    esm1p5_output_dir: an "outputXYZ" directory produced by an ESM1.5 simulation. 
-        Fields files in the "atmosphere" subdirectory will be converted to NetCDF.
+    esm1p5_output_dir: an "outputXYZ" directory produced by an ESM1.5 simulation.
+            Fields files in the "atmosphere" subdirectory will be
+            converted to NetCDF.
+    delete_ff : Delete fields files upon successful conversion. when true
 
     Returns
     -------
@@ -274,7 +280,8 @@ def convert_esm1p5_output_dir(esm1p5_output_dir):
 
     succeeded, failed = convert_fields_file_list(
         atm_dir_fields_files,
-        nc_write_dir
+        nc_write_dir,
+        delete_ff
     )
 
     return succeeded, failed
@@ -292,11 +299,15 @@ def convert_esm1p5_output_dir(esm1p5_output_dir):
                             "Otherwise report full stack trace."
                         )
                         )
+    parser.add_argument("--delete", "-d", action="store_true",
+                        help="Delete fields files upon successful conversion"
+                        )
     args = parser.parse_args()
 
     current_output_dir = args.current_output_dir
 
-    successes, failures = convert_esm1p5_output_dir(current_output_dir)
+    successes, failures = convert_esm1p5_output_dir(current_output_dir,
+                                                    args.delete)
 
     # Report results to user
     for success_message in format_successes(successes):

From d56525e4f027b076285507c67157bc2815d204b6 Mon Sep 17 00:00:00 2001
From: Spencer Wong <spencer.wong@anu.edu.au>
Date: Sun, 18 Aug 2024 12:55:53 +1000
Subject: [PATCH 02/12] Naming clarity and linting

---
 umpost/conversion_driver_esm1p5.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/umpost/conversion_driver_esm1p5.py b/umpost/conversion_driver_esm1p5.py
index a4cdf99..fdab584 100755
--- a/umpost/conversion_driver_esm1p5.py
+++ b/umpost/conversion_driver_esm1p5.py
@@ -214,7 +214,7 @@ def format_failures(failed, quiet):
             )
             yield failure_report
     else:
-        
+
         for fields_file_path, exception in failed:
             formatted_traceback = "".join(
                 traceback.format_exception(exception)
@@ -299,7 +299,7 @@ def convert_esm1p5_output_dir(esm1p5_output_dir, delete_ff):
                             "Otherwise report full stack trace."
                         )
                         )
-    parser.add_argument("--delete", "-d", action="store_true",
+    parser.add_argument("--delete_ff", "-d", action="store_true",
                         help="Delete fields files upon successful conversion"
                         )
     args = parser.parse_args()
@@ -307,7 +307,7 @@ def convert_esm1p5_output_dir(esm1p5_output_dir, delete_ff):
     current_output_dir = args.current_output_dir
 
     successes, failures = convert_esm1p5_output_dir(current_output_dir,
-                                                    args.delete)
+                                                    args.delete_ff)
 
     # Report results to user
     for success_message in format_successes(successes):

From 243a768538449a53776adc77a8bd02185c4cd5a6 Mon Sep 17 00:00:00 2001
From: Spencer Wong <spencer.wong@anu.edu.au>
Date: Sun, 18 Aug 2024 14:52:19 +1000
Subject: [PATCH 03/12] Add tests for file removal

---
 test/test_conversion_driver_esm1p5.py | 51 ++++++++++++++++++++++-----
 umpost/conversion_driver_esm1p5.py    |  2 +-
 2 files changed, 44 insertions(+), 9 deletions(-)

diff --git a/test/test_conversion_driver_esm1p5.py b/test/test_conversion_driver_esm1p5.py
index 49071a3..c20b035 100644
--- a/test/test_conversion_driver_esm1p5.py
+++ b/test/test_conversion_driver_esm1p5.py
@@ -102,6 +102,11 @@ def mock_process(base_mock_process):
     base_mock_process.return_value = None
     return base_mock_process
 
+@pytest.fixture
+def mock_os_remove():
+    patcher = mock.patch("os.remove")
+    yield patcher.start()
+    patcher.stop()
 
 @pytest.fixture
 def mock_process_with_exception(mock_process):
@@ -118,20 +123,43 @@ def _mock_process_with_exception(error_message):
     "input_list", [[], ["fake_file"], [
         "fake_file_1", "fake_file_2", "fake_file_3"]]
 )
-def test_convert_fields_file_list_success(mock_process, input_list):
+def test_convert_fields_file_list_success_delete(mock_process,
+                                                 mock_os_remove,
+                                                 input_list):
+    """
+    Test that process is called for each input and that each input is deleted.
+    """
     input_list_paths = [Path(p) for p in input_list]
+
     succeeded, _ = esm1p5_convert.convert_fields_file_list(
-        input_list_paths, "fake_nc_write_dir")
+        input_list_paths, "fake_nc_write_dir", delete_ff=True)
 
     assert mock_process.call_count == len(input_list)
+    assert mock_os_remove.call_count == len(input_list)
+
+    for path in input_list_paths:
+        assert mock.call(path) in mock_os_remove.call_args_list
 
     successful_input_paths = [successful_path_pair[0] for
                               successful_path_pair in succeeded]
-    
+
     assert input_list_paths == successful_input_paths
 
 
-def test_convert_fields_file_list_fail_excepted(mock_process_with_exception):
+def test_convert_fields_file_list_no_delete(mock_process, mock_os_remove):
+    """
+    Test that files are not deleted when delete_ff is False.
+    """
+    input_list_paths = [Path("fake_file_1"), Path("fake_file_2")]
+
+    succeeded, _ = esm1p5_convert.convert_fields_file_list(
+        input_list_paths, "fake_nc_write_dir", delete_ff=False)
+
+    mock_os_remove.assert_not_called()
+
+
+def test_convert_fields_file_list_fail_excepted(mock_process_with_exception,
+                                                mock_os_remove):
     # Hopefully this test will be unnecessary with um2nc standalone.
     # Test that the "Variable can not be processed" error arising from time
     # series inputs is excepted.
@@ -142,30 +170,37 @@ def test_convert_fields_file_list_fail_excepted(mock_process_with_exception):
     fake_file_path = Path("fake_file")
 
     _, failed = esm1p5_convert.convert_fields_file_list(
-        [fake_file_path], "fake_nc_write_dir")
+        [fake_file_path], "fake_nc_write_dir", delete_ff=True)
 
     assert failed[0][0] == fake_file_path
 
+    # Assert that no files removed
+    mock_os_remove.assert_not_called()
+
     # TODO: Testing the exception part of the reported failures will be easier
     # once um2nc specific exceptions are added.
 
 
-def test_convert_fields_file_list_fail_critical(mock_process_with_exception):
+def test_convert_fields_file_list_fail_critical(mock_process_with_exception, mock_os_remove):
     # Test that critical exceptions which are not allowed by ALLOWED_UM2NC_EXCEPTION_MESSAGES
     # are raised, and hence lead to the conversion crashing.
     generic_error_message = "Test error"
     mock_process_with_exception(generic_error_message)
     with pytest.raises(Exception) as exc_info:
         esm1p5_convert.convert_fields_file_list(
-            ["fake_file"], "fake_nc_write_dir")
+            ["fake_file"], "fake_nc_write_dir", delete_ff=True)
 
     assert str(exc_info.value) == generic_error_message
 
+    # Assert that no files removed
+    mock_os_remove.assert_not_called()
+
 
 def test_convert_esm1p5_output_dir_error():
     with pytest.raises(FileNotFoundError):
         esm1p5_convert.convert_esm1p5_output_dir(
-            "/test_convert_esm1p5_output_dir_error/fake/path/"
+            "/test_convert_esm1p5_output_dir_error/fake/path/",
+            delete_ff=True
         )
 
 
diff --git a/umpost/conversion_driver_esm1p5.py b/umpost/conversion_driver_esm1p5.py
index fdab584..ca0c146 100755
--- a/umpost/conversion_driver_esm1p5.py
+++ b/umpost/conversion_driver_esm1p5.py
@@ -157,7 +157,7 @@ def convert_fields_file_list(fields_file_paths, nc_write_dir, delete_ff):
             um2netcdf4.process(fields_file_path, nc_write_path, ARG_VALS)
             succeeded.append((fields_file_path, nc_write_path))
             if delete_ff:
-                fields_file_path.unlink()
+                os.remove(fields_file_path)
 
         except Exception as exc:
             # TODO: Refactor once um2nc has specific exceptions

From 379001b7918487ee3e3a1eb05b5c523a13350b8f Mon Sep 17 00:00:00 2001
From: Spencer Wong <spencer.wong@anu.edu.au>
Date: Mon, 19 Aug 2024 11:28:49 +1000
Subject: [PATCH 04/12] Typos and linting

---
 test/test_conversion_driver_esm1p5.py | 4 ++--
 umpost/conversion_driver_esm1p5.py    | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/test/test_conversion_driver_esm1p5.py b/test/test_conversion_driver_esm1p5.py
index c20b035..117d501 100644
--- a/test/test_conversion_driver_esm1p5.py
+++ b/test/test_conversion_driver_esm1p5.py
@@ -255,11 +255,11 @@ def test_format_failures_standard_mode():
     try:
         raise exception_2 from exception_1
     except Exception as exc:
-        exc_with_traceback = exc 
+        exc_with_traceback = exc
 
     failed_file = Path("fake_file")
     failed_conversion = [(failed_file, exc_with_traceback)]
-    
+
     formatted_failure_report_list = list(
         esm1p5_convert.format_failures(failed_conversion, quiet=False)
     )
diff --git a/umpost/conversion_driver_esm1p5.py b/umpost/conversion_driver_esm1p5.py
index ca0c146..13c3d75 100755
--- a/umpost/conversion_driver_esm1p5.py
+++ b/umpost/conversion_driver_esm1p5.py
@@ -300,7 +300,7 @@ def convert_esm1p5_output_dir(esm1p5_output_dir, delete_ff):
                         )
                         )
     parser.add_argument("--delete_ff", "-d", action="store_true",
-                        help="Delete fields files upon successful conversion"
+                        help="Delete fields files upon successful conversion."
                         )
     args = parser.parse_args()
 

From 11afdca367a725d5e50a11659f7a4a6f017a340a Mon Sep 17 00:00:00 2001
From: Spencer Wong <spencer.wong@anu.edu.au>
Date: Tue, 20 Aug 2024 12:36:25 +1000
Subject: [PATCH 05/12] Update argparser deletion argument

---
 umpost/conversion_driver_esm1p5.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/umpost/conversion_driver_esm1p5.py b/umpost/conversion_driver_esm1p5.py
index 13c3d75..934ae96 100755
--- a/umpost/conversion_driver_esm1p5.py
+++ b/umpost/conversion_driver_esm1p5.py
@@ -299,7 +299,7 @@ def convert_esm1p5_output_dir(esm1p5_output_dir, delete_ff):
                             "Otherwise report full stack trace."
                         )
                         )
-    parser.add_argument("--delete_ff", "-d", action="store_true",
+    parser.add_argument("--delete-ff", "-d", action="store_true",
                         help="Delete fields files upon successful conversion."
                         )
     args = parser.parse_args()

From 2cf7107a54e6b8eb0c1e758781d5f05b01435641 Mon Sep 17 00:00:00 2001
From: Spencer Wong <spencer.wong@anu.edu.au>
Date: Tue, 20 Aug 2024 15:22:59 +1000
Subject: [PATCH 06/12] Move file deletion up to main

---
 umpost/conversion_driver_esm1p5.py | 37 +++++++++++++++++++++---------
 1 file changed, 26 insertions(+), 11 deletions(-)

diff --git a/umpost/conversion_driver_esm1p5.py b/umpost/conversion_driver_esm1p5.py
index 934ae96..a362611 100755
--- a/umpost/conversion_driver_esm1p5.py
+++ b/umpost/conversion_driver_esm1p5.py
@@ -126,7 +126,7 @@ def find_matching_fields_files(dir_contents, fields_file_name_pattern):
     return fields_file_paths
 
 
-def convert_fields_file_list(fields_file_paths, nc_write_dir, delete_ff):
+def convert_fields_file_list(fields_file_paths, nc_write_dir):
     """
     Convert group of fields files to NetCDF, writing output in nc_write_dir.
 
@@ -134,8 +134,6 @@ def convert_fields_file_list(fields_file_paths, nc_write_dir, delete_ff):
     ----------
     fields_file_paths : list of paths to fields files for conversion.
     nc_write_dir : directory to save NetCDF files into.
-    delete_ff : Boolean specifying whether to delete fields files upon
-                  successful conversion.
 
     Returns
     -------
@@ -156,8 +154,6 @@ def convert_fields_file_list(fields_file_paths, nc_write_dir, delete_ff):
         try:
             um2netcdf4.process(fields_file_path, nc_write_path, ARG_VALS)
             succeeded.append((fields_file_path, nc_write_path))
-            if delete_ff:
-                os.remove(fields_file_path)
 
         except Exception as exc:
             # TODO: Refactor once um2nc has specific exceptions
@@ -226,7 +222,7 @@ def format_failures(failed, quiet):
             yield failure_report
 
 
-def convert_esm1p5_output_dir(esm1p5_output_dir, delete_ff):
+def convert_esm1p5_output_dir(esm1p5_output_dir):
     """
     Driver function for converting ESM1.5 atmospheric outputs during a simulation.
 
@@ -235,7 +231,6 @@ def convert_esm1p5_output_dir(esm1p5_output_dir, delete_ff):
     esm1p5_output_dir: an "outputXYZ" directory produced by an ESM1.5 simulation.
             Fields files in the "atmosphere" subdirectory will be
             converted to NetCDF.
-    delete_ff : Delete fields files upon successful conversion. when true
 
     Returns
     -------
@@ -280,12 +275,19 @@ def convert_esm1p5_output_dir(esm1p5_output_dir, delete_ff):
 
     succeeded, failed = convert_fields_file_list(
         atm_dir_fields_files,
-        nc_write_dir,
-        delete_ff
+        nc_write_dir
     )
 
     return succeeded, failed
 
+def success_fail_overlap(succeeded, failed):
+    succeeded_inputs = [succeed_path for succeed_path, _ in succeeded]
+    failed_inputs = [fail_path for fail_path, _ in failed]
+
+    overlap = set(succeeded_inputs) & set(failed_inputs)
+
+    return overlap
+
 
 if __name__ == "__main__":
     parser = argparse.ArgumentParser()
@@ -306,11 +308,24 @@ def convert_esm1p5_output_dir(esm1p5_output_dir, delete_ff):
 
     current_output_dir = args.current_output_dir
 
-    successes, failures = convert_esm1p5_output_dir(current_output_dir,
-                                                    args.delete_ff)
+    successes, failures = convert_esm1p5_output_dir(current_output_dir)
 
     # Report results to user
     for success_message in format_successes(successes):
         print(success_message)
     for failure_message in format_failures(failures, args.quiet):
         warnings.warn(failure_message)
+
+    if args.delete_ff:
+        # Check that no successful inputs somehow simultaneously failed.
+        overlap = success_fail_overlap(successes, failures)
+        if overlap: 
+            msg = (
+                    "Following inputs reported simultaneous successful and "
+                    "failed conversions. Inputs will not be deleted.\n"
+                    f"{overlap}"
+            )
+            raise um2netcdf.PostProcessingError()
+        else:
+            for successful_input_path, _ in successes:
+                os.remove(successful_input_path)

From b86c0b2820610e0b6fb6ab04a708dfa79488b1b4 Mon Sep 17 00:00:00 2001
From: Spencer Wong <spencer.wong@anu.edu.au>
Date: Tue, 20 Aug 2024 15:27:09 +1000
Subject: [PATCH 07/12] linting

---
 umpost/conversion_driver_esm1p5.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/umpost/conversion_driver_esm1p5.py b/umpost/conversion_driver_esm1p5.py
index e114632..bf01a1d 100755
--- a/umpost/conversion_driver_esm1p5.py
+++ b/umpost/conversion_driver_esm1p5.py
@@ -310,7 +310,7 @@ def success_fail_overlap(succeeded, failed):
     if args.delete_ff:
         # Check that no successful inputs somehow simultaneously failed.
         overlap = success_fail_overlap(successes, failures)
-        if overlap: 
+        if overlap:
             msg = (
                     "Following inputs reported simultaneous successful and "
                     "failed conversions. Inputs will not be deleted.\n"

From 9045765afb7a8d6e6c62159b53b640d8e7bd5b36 Mon Sep 17 00:00:00 2001
From: Spencer Wong <spencer.wong@anu.edu.au>
Date: Tue, 20 Aug 2024 15:34:11 +1000
Subject: [PATCH 08/12] Add message to overlap error

---
 umpost/conversion_driver_esm1p5.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/umpost/conversion_driver_esm1p5.py b/umpost/conversion_driver_esm1p5.py
index bf01a1d..b8dc545 100755
--- a/umpost/conversion_driver_esm1p5.py
+++ b/umpost/conversion_driver_esm1p5.py
@@ -316,7 +316,7 @@ def success_fail_overlap(succeeded, failed):
                     "failed conversions. Inputs will not be deleted.\n"
                     f"{overlap}"
             )
-            raise um2netcdf.PostProcessingError()
+            raise um2netcdf.PostProcessingError(msg)
         else:
             for successful_input_path, _ in successes:
                 os.remove(successful_input_path)

From c53c422f3a4e790665f29ab9dd014c526b83cb08 Mon Sep 17 00:00:00 2001
From: Spencer Wong <spencer.wong@anu.edu.au>
Date: Tue, 20 Aug 2024 15:40:47 +1000
Subject: [PATCH 09/12] Fix tests impacted by moving deletion

---
 test/test_conversion_driver_esm1p5.py | 48 +++++----------------------
 1 file changed, 9 insertions(+), 39 deletions(-)

diff --git a/test/test_conversion_driver_esm1p5.py b/test/test_conversion_driver_esm1p5.py
index 981aab7..948aa6f 100644
--- a/test/test_conversion_driver_esm1p5.py
+++ b/test/test_conversion_driver_esm1p5.py
@@ -95,11 +95,6 @@ def mock_process(base_mock_process):
     base_mock_process.return_value = None
     return base_mock_process
 
-@pytest.fixture
-def mock_os_remove():
-    patcher = mock.patch("os.remove")
-    yield patcher.start()
-    patcher.stop()
 
 @pytest.fixture
 def mock_process_with_exception(mock_process):
@@ -116,22 +111,17 @@ def _mock_process_with_exception(error_message):
     "input_list", [[], ["fake_file"], [
         "fake_file_1", "fake_file_2", "fake_file_3"]]
 )
-def test_convert_fields_file_list_success_delete(mock_process,
-                                                 mock_os_remove,
-                                                 input_list):
+def test_convert_fields_file_list_success(mock_process,
+                                          input_list):
     """
-    Test that process is called for each input and that each input is deleted.
+    Test that process is called for each input.
     """
     input_list_paths = [Path(p) for p in input_list]
 
     succeeded, _ = esm1p5_convert.convert_fields_file_list(
-        input_list_paths, "fake_nc_write_dir", delete_ff=True)
+        input_list_paths, "fake_nc_write_dir")
 
     assert mock_process.call_count == len(input_list)
-    assert mock_os_remove.call_count == len(input_list)
-
-    for path in input_list_paths:
-        assert mock.call(path) in mock_os_remove.call_args_list
 
     successful_input_paths = [successful_path_pair[0] for
                               successful_path_pair in succeeded]
@@ -139,20 +129,7 @@ def test_convert_fields_file_list_success_delete(mock_process,
     assert input_list_paths == successful_input_paths
 
 
-def test_convert_fields_file_list_no_delete(mock_process, mock_os_remove):
-    """
-    Test that files are not deleted when delete_ff is False.
-    """
-    input_list_paths = [Path("fake_file_1"), Path("fake_file_2")]
-
-    succeeded, _ = esm1p5_convert.convert_fields_file_list(
-        input_list_paths, "fake_nc_write_dir", delete_ff=False)
-
-    mock_os_remove.assert_not_called()
-
-
-def test_convert_fields_file_list_fail_excepted(mock_process_with_exception,
-                                                mock_os_remove):
+def test_convert_fields_file_list_fail_excepted(mock_process_with_exception):
     # Hopefully this test will be unnecessary with um2nc standalone.
     # Test that the "Variable can not be processed" error arising from time
     # series inputs is excepted.
@@ -163,37 +140,30 @@ def test_convert_fields_file_list_fail_excepted(mock_process_with_exception,
     fake_file_path = Path("fake_file")
 
     _, failed = esm1p5_convert.convert_fields_file_list(
-        [fake_file_path], "fake_nc_write_dir", delete_ff=True)
+        [fake_file_path], "fake_nc_write_dir")
 
     assert failed[0][0] == fake_file_path
 
-    # Assert that no files removed
-    mock_os_remove.assert_not_called()
-
     # TODO: Testing the exception part of the reported failures will be easier
     # once um2nc specific exceptions are added.
 
 
-def test_convert_fields_file_list_fail_critical(mock_process_with_exception, mock_os_remove):
+def test_convert_fields_file_list_fail_critical(mock_process_with_exception):
     # Test that critical exceptions which are not allowed by ALLOWED_UM2NC_EXCEPTION_MESSAGES
     # are raised, and hence lead to the conversion crashing.
     generic_error_message = "Test error"
     mock_process_with_exception(generic_error_message)
     with pytest.raises(Exception) as exc_info:
         esm1p5_convert.convert_fields_file_list(
-            ["fake_file"], "fake_nc_write_dir", delete_ff=True)
+            ["fake_file"], "fake_nc_write_dir")
 
     assert str(exc_info.value) == generic_error_message
 
-    # Assert that no files removed
-    mock_os_remove.assert_not_called()
-
 
 def test_convert_esm1p5_output_dir_error():
     with pytest.raises(FileNotFoundError):
         esm1p5_convert.convert_esm1p5_output_dir(
-            "/test_convert_esm1p5_output_dir_error/fake/path/",
-            delete_ff=True
+            "/test_convert_esm1p5_output_dir_error/fake/path/"
         )
 
 

From 16e1e3905dc688f980aa335a644dede9dae60c6b Mon Sep 17 00:00:00 2001
From: Spencer Wong <spencer.wong@anu.edu.au>
Date: Tue, 20 Aug 2024 16:49:22 +1000
Subject: [PATCH 10/12] Update success vs faliure checks for file deletion

---
 test/test_conversion_driver_esm1p5.py | 15 +++++++++++
 umpost/conversion_driver_esm1p5.py    | 39 +++++++++++++++------------
 2 files changed, 37 insertions(+), 17 deletions(-)

diff --git a/test/test_conversion_driver_esm1p5.py b/test/test_conversion_driver_esm1p5.py
index 948aa6f..3f7684d 100644
--- a/test/test_conversion_driver_esm1p5.py
+++ b/test/test_conversion_driver_esm1p5.py
@@ -233,3 +233,18 @@ def test_format_failures_standard_mode():
 
     assert exception_1.args[0] in formatted_failure_report
     assert exception_2.args[0] in formatted_failure_report
+
+
+def test_success_fail_overlap():
+    # Test that inputs listed as both successes and failures
+    # are removed as candidates for deletion.
+    success_only_path = Path("success_only")
+    success_and_fail_path = Path("success_and_fail")
+    successes = [(success_only_path, Path("success_only.nc")),
+                 (success_and_fail_path, Path("success_and_fail.nc"))]
+    failures = [(success_and_fail_path, "Exception_placeholder")]
+
+    result = esm1p5_convert.safe_removal(successes, failures)
+
+    assert success_and_fail_path not in result
+    assert success_only_path in result
diff --git a/umpost/conversion_driver_esm1p5.py b/umpost/conversion_driver_esm1p5.py
index b8dc545..28697ee 100755
--- a/umpost/conversion_driver_esm1p5.py
+++ b/umpost/conversion_driver_esm1p5.py
@@ -271,13 +271,27 @@ def convert_esm1p5_output_dir(esm1p5_output_dir):
 
     return succeeded, failed
 
-def success_fail_overlap(succeeded, failed):
-    succeeded_inputs = [succeed_path for succeed_path, _ in succeeded]
-    failed_inputs = [fail_path for fail_path, _ in failed]
+def safe_removal(succeeded, failed):
+    """
+    Check whether any input files were reported as simultaneously
+    successful and failed conversions. Return those that appear
+    only as successes as targets for safe deletion.
+
+    Parameters
+    ----------
+    succeeded: List of (input_file, output_file) tuples of filepaths from
+        successful conversions.
+    failed: List of (input_file, Exception) tuples from failed conversions.
 
-    overlap = set(succeeded_inputs) & set(failed_inputs)
+    Returns
+    -------
+    successful_only: set of input filepaths which appear in succeeded but
+        not failed.
+    """
+    succeeded_inputs = {succeed_path for succeed_path, _ in succeeded}
+    failed_inputs = {fail_path for fail_path, _ in failed}
 
-    return overlap
+    return succeeded_inputs - failed_inputs
 
 
 if __name__ == "__main__":
@@ -308,15 +322,6 @@ def success_fail_overlap(succeeded, failed):
         warnings.warn(failure_message)
 
     if args.delete_ff:
-        # Check that no successful inputs somehow simultaneously failed.
-        overlap = success_fail_overlap(successes, failures)
-        if overlap:
-            msg = (
-                    "Following inputs reported simultaneous successful and "
-                    "failed conversions. Inputs will not be deleted.\n"
-                    f"{overlap}"
-            )
-            raise um2netcdf.PostProcessingError(msg)
-        else:
-            for successful_input_path, _ in successes:
-                os.remove(successful_input_path)
+        # Remove files that appear only as successful conversions
+        for path in safe_removal():
+            os.remove(path)

From df5b450ab436a29cfd905848f7b21b2de9d9be76 Mon Sep 17 00:00:00 2001
From: Spencer Wong <spencer.wong@anu.edu.au>
Date: Tue, 20 Aug 2024 16:57:07 +1000
Subject: [PATCH 11/12] fix missing args

---
 umpost/conversion_driver_esm1p5.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/umpost/conversion_driver_esm1p5.py b/umpost/conversion_driver_esm1p5.py
index 28697ee..ad0310b 100755
--- a/umpost/conversion_driver_esm1p5.py
+++ b/umpost/conversion_driver_esm1p5.py
@@ -323,5 +323,5 @@ def safe_removal(succeeded, failed):
 
     if args.delete_ff:
         # Remove files that appear only as successful conversions
-        for path in safe_removal():
+        for path in safe_removal(successes, failures):
             os.remove(path)

From 0e51a1314e689f80023e4962a2168d5bcbc61a37 Mon Sep 17 00:00:00 2001
From: Spencer Wong <spencer.wong@anu.edu.au>
Date: Tue, 20 Aug 2024 17:13:42 +1000
Subject: [PATCH 12/12] Missing newline

---
 umpost/conversion_driver_esm1p5.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/umpost/conversion_driver_esm1p5.py b/umpost/conversion_driver_esm1p5.py
index ad0310b..d7cbe9f 100755
--- a/umpost/conversion_driver_esm1p5.py
+++ b/umpost/conversion_driver_esm1p5.py
@@ -271,6 +271,7 @@ def convert_esm1p5_output_dir(esm1p5_output_dir):
 
     return succeeded, failed
 
+
 def safe_removal(succeeded, failed):
     """
     Check whether any input files were reported as simultaneously