From afa8d1ceb74bcb8cf10a65ec631b438cc24bf31a Mon Sep 17 00:00:00 2001 From: Nakul Santhosh Date: Fri, 10 Jul 2026 14:19:23 +0200 Subject: [PATCH 1/6] feat: add cleanup routines for files in working directory --- src/opi/output/core.py | 49 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 48 insertions(+), 1 deletion(-) diff --git a/src/opi/output/core.py b/src/opi/output/core.py index 23abdbbc..5defa834 100644 --- a/src/opi/output/core.py +++ b/src/opi/output/core.py @@ -4,7 +4,7 @@ """ import json -from collections.abc import Callable, Sequence +from collections.abc import Callable, Iterable, Sequence from pathlib import Path from typing import Any, cast from warnings import warn @@ -513,6 +513,53 @@ def get_file(self, suffix: str, /, *, basename: str = "") -> Path: basename = self.basename return self.working_dir / (basename + suffix) + def _delete_files(self, basename: str | None, *, suffixes: Sequence[str] | None = None) -> None: + """ + Delete files in `working_dir` belonging to the job with the given basename. + + Parameters + ---------- + basename : str | None + Basename of the job whose files should be deleted. + If not given, `self.basename` is used. + suffixes : Sequence[str] | None, default: None + If given, only files with these suffixes are deleted. + If None, all files matching the basename are deleted. + """ + basename = basename if basename is not None else self.basename + files: Iterable[Path] + if suffixes is None: + files = self.working_dir.glob(f"{basename}*") + else: + files = (self.get_file(suffix, basename=basename) for suffix in suffixes) + for file in files: + file.unlink(missing_ok=True) + + def cleanup_files(self, basename: str | None = None) -> None: + """ + Delete all files in `working_dir` belonging to the job with the given basename. + + Parameters + ---------- + basename : str | None, default: None + Basename of the job whose files should be deleted. + If not given, `self.basename` is used. + """ + self._delete_files(basename) + + def cleanup_temp_files(self, basename: str | None = None) -> None: + """ + Delete temporary files in `working_dir` belonging to the job with the given basename. + + Parameters + ---------- + basename : str | None, default: None + Basename of the job whose temporary files should be deleted. + If not given, `self.basename` is used. + """ + temp_file_suffixes: tuple[str, ...] = (".tmp", ".proc") + self._delete_files(basename, suffixes=temp_file_suffixes) + def _get_version(self) -> "OrcaVersion": """Gets the ORCA version from the property-JSON file.""" From 1da28e13a5b94ccbb8125c685c1422d6663a1bdb Mon Sep 17 00:00:00 2001 From: Nakul Santhosh Date: Mon, 13 Jul 2026 14:31:54 +0200 Subject: [PATCH 2/6] fix: add check for basename --- src/opi/output/core.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/opi/output/core.py b/src/opi/output/core.py index 5defa834..40b2adc3 100644 --- a/src/opi/output/core.py +++ b/src/opi/output/core.py @@ -527,6 +527,8 @@ def _delete_files(self, basename: str | None, *, suffixes: Sequence[str] | None If None, all files matching the basename are deleted. """ basename = basename if basename is not None else self.basename + if basename is None: + raise ValueError("No basename specified") files: Iterable[Path] if suffixes is None: files = self.working_dir.glob(f"{basename}*") From 4653a125a1c04ef4f58d3c2b1dbb071a923a2423 Mon Sep 17 00:00:00 2001 From: Nakul Santhosh Date: Mon, 13 Jul 2026 14:35:29 +0200 Subject: [PATCH 3/6] fix:add to changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f0ec98bd..dd1110c8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ - Add the keyword `dumpactints` to `BlockOutput` (#245). - Added fallback keyword argument for `get_final_energy`, `get_gradient`, and `get_structure` that allows to parse these properties from the `.out` file if no JSON output is available (#237). - Added `get_frequencies`, `get_imaginary_frequencies`, `is_pes_minimum`, and `is_pes_transition_state` to the `Output` class (#247). +- Add `functions to clean up files created by ORCA jobs.(#262) ### Changed - Refactored methods from Runner into BaseRunner (#193) From f11bd771f84fb9c19aafe839e2ae0011e0e7f288 Mon Sep 17 00:00:00 2001 From: Nakul Santhosh Date: Mon, 13 Jul 2026 15:28:41 +0200 Subject: [PATCH 4/6] fix: add truthiness checks for basename and checks for files to be deleted --- src/opi/output/core.py | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/opi/output/core.py b/src/opi/output/core.py index 40b2adc3..9ea670e7 100644 --- a/src/opi/output/core.py +++ b/src/opi/output/core.py @@ -523,23 +523,31 @@ def _delete_files(self, basename: str | None, *, suffixes: Sequence[str] | None Basename of the job whose files should be deleted. If not given, `self.basename` is used. suffixes : Sequence[str] | None, default: None - If given, only files with these suffixes are deleted. + If given, treated as glob patterns matched against `{basename}*{suffix}`; + only matching files are deleted. If None, all files matching the basename are deleted. """ basename = basename if basename is not None else self.basename - if basename is None: + if not basename: raise ValueError("No basename specified") files: Iterable[Path] if suffixes is None: files = self.working_dir.glob(f"{basename}*") else: - files = (self.get_file(suffix, basename=basename) for suffix in suffixes) + files = ( + file + for suffix in suffixes + for file in self.working_dir.glob(f"{basename}*{suffix}*") + ) for file in files: - file.unlink(missing_ok=True) + if file.is_file(): + file.unlink(missing_ok=True) def cleanup_files(self, basename: str | None = None) -> None: """ - Delete all files in `working_dir` belonging to the job with the given basename. + Delete all files in `working_dir` belonging to the job with the given basename. This will also include files whose + names contain the basename, for example, if the basename is `job` and there exists files with basename 'job_1', + the files will be deleted. Parameters ---------- From 033a54b996da4e3f6600cb01464df28ac69b27f6 Mon Sep 17 00:00:00 2001 From: Nakul Santhosh Date: Wed, 15 Jul 2026 15:53:53 +0200 Subject: [PATCH 5/6] fix: add safeguard against relative paths when specifying basenames for cleanup functions --- src/opi/output/core.py | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/opi/output/core.py b/src/opi/output/core.py index d29774e9..09ebfc9a 100644 --- a/src/opi/output/core.py +++ b/src/opi/output/core.py @@ -536,13 +536,15 @@ def get_file(self, suffix: str, /, *, basename: str = "") -> Path: basename = self.basename return self.working_dir / (basename + suffix) - def _delete_files(self, basename: str | None, *, suffixes: Sequence[str] | None = None) -> None: + def _delete_files( + self, basename: str | None = None, *, suffixes: Sequence[str] = tuple() + ) -> None: """ Delete files in `working_dir` belonging to the job with the given basename. Parameters ---------- - basename : str | None + basename : str | None, default: None Basename of the job whose files should be deleted. If not given, `self.basename` is used. suffixes : Sequence[str] | None, default: None @@ -550,17 +552,19 @@ def _delete_files(self, basename: str | None, *, suffixes: Sequence[str] | None only matching files are deleted. If None, all files matching the basename are deleted. """ - basename = basename if basename is not None else self.basename + basename = basename if basename else self.basename if not basename: raise ValueError("No basename specified") + if "/" in basename or "\\" in basename: + raise ValueError(f"Basename must not contain path separators: {basename!r}") files: Iterable[Path] - if suffixes is None: + if not suffixes: files = self.working_dir.glob(f"{basename}*") else: files = ( file for suffix in suffixes - for file in self.working_dir.glob(f"{basename}*{suffix}*") + for file in self.working_dir.glob(f"{basename}*{suffix}") ) for file in files: if file.is_file(): @@ -590,7 +594,7 @@ def cleanup_temp_files(self, basename: str | None = None) -> None: Basename of the job whose temporary files should be deleted. If not given, `self.basename` is used. """ - temp_file_suffixes: tuple[str, ...] = (".tmp", ".proc") + temp_file_suffixes: tuple[str, ...] = (".tmp", ".proc", ".tmp.*", ".proc.*") self._delete_files(basename, suffixes=temp_file_suffixes) def _get_version(self) -> "OrcaVersion": From ec8f2bb1a9d7aeb4ec10c6db2337b1dcbe90097e Mon Sep 17 00:00:00 2001 From: Nakul Santhosh Date: Wed, 15 Jul 2026 15:54:23 +0200 Subject: [PATCH 6/6] fix: add unit tests for cleanup functions --- tests/unit/test_output_cleanup.py | 108 ++++++++++++++++++++++++++++++ 1 file changed, 108 insertions(+) create mode 100644 tests/unit/test_output_cleanup.py diff --git a/tests/unit/test_output_cleanup.py b/tests/unit/test_output_cleanup.py new file mode 100644 index 00000000..9a0b313d --- /dev/null +++ b/tests/unit/test_output_cleanup.py @@ -0,0 +1,108 @@ +from pathlib import Path + +import pytest + +from opi.output.core import Output + +""" +Unit tests for Output cleanup functions. + +This module contains tests for the file-deletion helpers: +- `Output.cleanup_files()` +- `Output.cleanup_temp_files()` +- `Output._delete_files()` +""" + + +def _make_output(basename: str, working_dir: Path) -> Output: + output = Output(basename, working_dir=working_dir, version_check=False) + return output + + +def _create_file(directory: Path, *names: str) -> None: + for name in names: + (directory / name).touch() + + +@pytest.mark.unit +@pytest.mark.output +def test_cleanup_files_deletes_matching_basename_files(tmp_path: Path): + """`cleanup_files()` deletes all files matching the basename, including files whose + name only contains the basename as a prefix, and leaves unrelated files untouched.""" + _create_file(tmp_path, "job.out", "job.gbw", "job_1.gbw", "unrelated.txt") + output = _make_output("job", tmp_path) + + output.cleanup_files() + + remaining = {p.name for p in tmp_path.iterdir()} + assert remaining == {"unrelated.txt"} + + +@pytest.mark.unit +@pytest.mark.output +def test_cleanup_files_explicit_basename_overrides_self_basename(tmp_path: Path): + """`cleanup_files(basename=...)` deletes files for the given basename, not `self.basename`.""" + _create_file(tmp_path, "job_1.out", "job_2.out") + output = _make_output("job_1", tmp_path) + + output.cleanup_files(basename="job_2") + + remaining = {p.name for p in tmp_path.iterdir()} + assert remaining == {"job_1.out"} + + +@pytest.mark.unit +@pytest.mark.output +def test_cleanup_files_raises_without_basename(tmp_path: Path): + """`cleanup_files()` raises `ValueError` rather than deleting everything in `working_dir`.""" + _create_file(tmp_path, "unrelated.txt") + output = _make_output("", tmp_path) + + with pytest.raises(ValueError, match="No basename specified"): + output.cleanup_files() + + assert (tmp_path / "unrelated.txt").exists() + + +@pytest.mark.unit +@pytest.mark.output +@pytest.mark.parametrize("basename", ["../secret", "sub/job", "sub\\job"]) +def test_cleanup_files_rejects_path_separators_in_basename(tmp_path: Path, basename: str): + """A basename containing a path separator must be rejected rather than letting the + glob pattern escape `working_dir`.""" + working_dir = tmp_path / "workdir" + working_dir.mkdir() + _create_file(tmp_path, "secret_file.txt") + output = _make_output(basename, working_dir) + + with pytest.raises(ValueError, match="path separators"): + output.cleanup_files() + + assert (tmp_path / "secret_file.txt").exists() + + +@pytest.mark.unit +@pytest.mark.output +def test_cleanup_temp_files_deletes_only_temp_suffixes(tmp_path: Path): + """`cleanup_temp_files()` only deletes `.tmp.*`/`.proc.*` files, not other job files.""" + _create_file(tmp_path, "job.out", "job.gbw", "job.tmp", "job.proc", "job_scan.tmp") + output = _make_output("job", tmp_path) + + output.cleanup_temp_files() + + remaining = {p.name for p in tmp_path.iterdir()} + assert remaining == {"job.out", "job.gbw"} + + +@pytest.mark.unit +@pytest.mark.output +def test_delete_files_empty_suffixes_deletes_all(tmp_path: Path): + """An empty `suffixes` sequence behaves like no filter at all: all files matching + the basename are deleted, same as omitting `suffixes`.""" + _create_file(tmp_path, "job.out", "job.gbw", "unrelated.txt") + output = _make_output("job", tmp_path) + + output._delete_files(None, suffixes=()) + + remaining = {p.name for p in tmp_path.iterdir()} + assert remaining == {"unrelated.txt"}