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] 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)