From 3ccd131d8ea3306ab1bb7593cd4d3b67b300c19a Mon Sep 17 00:00:00 2001 From: njzjz-bot Date: Mon, 27 Jul 2026 19:10:41 +0800 Subject: [PATCH 1/2] fix(cp2k): say what is missing from an AIMD output directory `cp2k/aimd_output` locates its trajectory and log by globbing the directory it is given, then subscripts the match list: xyz_file = sorted(glob.glob(f"{file_name}/*pos*.xyz"))[0] When nothing matches -- a run whose MOTION/PRINT/TRAJECTORY never wrote `-pos-1.xyz`, or a path pointing at a single file rather than its directory -- that raises a bare IndexError: list index out of range which names neither the pattern nor the directory. #673 is one report of it; answering that one took reading the plugin source to guess what the argument was supposed to be. Raise `FileNotFoundError` naming the pattern, the directory, what the directory actually contains, and how CP2K produces the file. A path that is not a directory says so directly. Fixes #673. Co-Authored-By: Claude Opus 5 --- dpdata/plugins/cp2k.py | 39 ++++++++++++++- tests/test_cp2k_aimd_missing_inputs.py | 68 ++++++++++++++++++++++++++ 2 files changed, 105 insertions(+), 2 deletions(-) create mode 100644 tests/test_cp2k_aimd_missing_inputs.py diff --git a/dpdata/plugins/cp2k.py b/dpdata/plugins/cp2k.py index 61f2eaf9..3b63df5b 100644 --- a/dpdata/plugins/cp2k.py +++ b/dpdata/plugins/cp2k.py @@ -1,6 +1,7 @@ from __future__ import annotations import glob +import os import dpdata.formats.cp2k.output from dpdata.format import Format @@ -16,11 +17,45 @@ """ +def _find_single_file(directory, pattern, description, hint): + """Resolve one file inside an AIMD output directory. + + ``cp2k/aimd_output`` is pointed at a directory and locates its inputs by + globbing. Without this, a directory missing one of them fails with a bare + ``IndexError`` from subscripting the empty match list, which says neither + what was looked for nor where. + """ + matches = sorted(glob.glob(os.path.join(directory, pattern))) + if not matches: + listing = sorted(os.listdir(directory)) if os.path.isdir(directory) else None + found = ( + f" The directory contains: {', '.join(listing) or '(nothing)'}." + if listing is not None + else f" {directory!r} is not a directory." + ) + raise FileNotFoundError( + f"cp2k/aimd_output found no {description} matching {pattern!r} in " + f"{directory!r}.{found} {hint}" + ) + return matches[0] + + @Format.register("cp2k/aimd_output") class CP2KAIMDOutputFormat(Format): def from_labeled_system(self, file_name, restart=False, **kwargs): - xyz_file = sorted(glob.glob(f"{file_name}/*pos*.xyz"))[0] - log_file = sorted(glob.glob(f"{file_name}/*.log"))[0] + xyz_file = _find_single_file( + file_name, + "*pos*.xyz", + "trajectory file", + "CP2K writes it as -pos-1.xyz when MOTION/PRINT/TRAJECTORY " + "is enabled; pass the directory holding it, not a single file.", + ) + log_file = _find_single_file( + file_name, + "*.log", + "output log", + "Redirect the CP2K stdout into this directory, e.g. cp2k.popt -i input.inp > cp2k.log.", + ) try: return tuple(Cp2kSystems(log_file, xyz_file, restart)) except (StopIteration, RuntimeError) as e: diff --git a/tests/test_cp2k_aimd_missing_inputs.py b/tests/test_cp2k_aimd_missing_inputs.py new file mode 100644 index 00000000..d8c1dad4 --- /dev/null +++ b/tests/test_cp2k_aimd_missing_inputs.py @@ -0,0 +1,68 @@ +from __future__ import annotations + +import os +import shutil +import tempfile +import unittest + +from context import dpdata + +FIXTURE = os.path.join("cp2k", "aimd") + + +class TestCp2kAimdMissingInputs(unittest.TestCase): + """``cp2k/aimd_output`` locates its inputs by globbing a directory. + + A directory missing one of them used to fail with a bare ``IndexError`` + from subscripting the empty match list. + """ + + def setUp(self): + self.tmp_dir = tempfile.mkdtemp() + self.work = os.path.join(self.tmp_dir, "aimd") + shutil.copytree(FIXTURE, self.work) + + def tearDown(self): + shutil.rmtree(self.tmp_dir, ignore_errors=True) + + def _remove(self, name): + os.remove(os.path.join(self.work, name)) + + def test_complete_directory_still_loads(self): + system = dpdata.LabeledSystem(self.work, fmt="cp2k/aimd_output") + self.assertGreater(system.get_nframes(), 0) + + def test_missing_trajectory_names_the_pattern(self): + self._remove("DPGEN-pos-1.xyz") + with self.assertRaises(FileNotFoundError) as caught: + dpdata.LabeledSystem(self.work, fmt="cp2k/aimd_output") + message = str(caught.exception) + self.assertIn("*pos*.xyz", message) + self.assertIn("trajectory file", message) + # The listing tells the user what the parser did see. + self.assertIn("cp2k.log", message) + + def test_missing_log_names_the_pattern(self): + self._remove("cp2k.log") + with self.assertRaises(FileNotFoundError) as caught: + dpdata.LabeledSystem(self.work, fmt="cp2k/aimd_output") + message = str(caught.exception) + self.assertIn("*.log", message) + self.assertIn("output log", message) + + def test_pointing_at_a_file_says_so(self): + # A frequent mistake: passing the log file rather than its directory. + log = os.path.join(self.work, "cp2k.log") + with self.assertRaises(FileNotFoundError) as caught: + dpdata.LabeledSystem(log, fmt="cp2k/aimd_output") + self.assertIn("is not a directory", str(caught.exception)) + + def test_missing_directory_says_so(self): + absent = os.path.join(self.tmp_dir, "absent") + with self.assertRaises(FileNotFoundError) as caught: + dpdata.LabeledSystem(absent, fmt="cp2k/aimd_output") + self.assertIn("is not a directory", str(caught.exception)) + + +if __name__ == "__main__": + unittest.main() From 34f6c535eaf2c1e3cb203ffc203cae3f96941747 Mon Sep 17 00:00:00 2001 From: njzjz-bot Date: Tue, 28 Jul 2026 21:16:05 +0800 Subject: [PATCH 2/2] fix(cp2k): address AIMD path review Escape glob metacharacters in directory names and distinguish missing paths from non-directory inputs. Coding-Agent: Codex Codex-Version: codex-cli 0.144.6 Model: gpt-5.6-sol Reasoning-Effort: xhigh --- dpdata/plugins/cp2k.py | 19 ++++++++++++------- tests/test_cp2k_aimd_missing_inputs.py | 8 +++++++- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/dpdata/plugins/cp2k.py b/dpdata/plugins/cp2k.py index 3b63df5b..d226253d 100644 --- a/dpdata/plugins/cp2k.py +++ b/dpdata/plugins/cp2k.py @@ -25,14 +25,19 @@ def _find_single_file(directory, pattern, description, hint): ``IndexError`` from subscripting the empty match list, which says neither what was looked for nor where. """ - matches = sorted(glob.glob(os.path.join(directory, pattern))) + # Only ``pattern`` is a glob. Escaping the directory keeps valid names + # such as ``run[1]`` from being interpreted as character classes. + matches = sorted( + glob.glob(os.path.join(glob.escape(os.fspath(directory)), pattern)) + ) if not matches: - listing = sorted(os.listdir(directory)) if os.path.isdir(directory) else None - found = ( - f" The directory contains: {', '.join(listing) or '(nothing)'}." - if listing is not None - else f" {directory!r} is not a directory." - ) + if not os.path.exists(directory): + found = f" {directory!r} does not exist." + elif not os.path.isdir(directory): + found = f" {directory!r} is not a directory." + else: + listing = sorted(os.listdir(directory)) + found = f" The directory contains: {', '.join(listing) or '(nothing)'}." raise FileNotFoundError( f"cp2k/aimd_output found no {description} matching {pattern!r} in " f"{directory!r}.{found} {hint}" diff --git a/tests/test_cp2k_aimd_missing_inputs.py b/tests/test_cp2k_aimd_missing_inputs.py index d8c1dad4..2e59b37b 100644 --- a/tests/test_cp2k_aimd_missing_inputs.py +++ b/tests/test_cp2k_aimd_missing_inputs.py @@ -32,6 +32,12 @@ def test_complete_directory_still_loads(self): system = dpdata.LabeledSystem(self.work, fmt="cp2k/aimd_output") self.assertGreater(system.get_nframes(), 0) + def test_directory_glob_metacharacters_are_literal(self): + bracketed = os.path.join(self.tmp_dir, "aimd[1]") + shutil.copytree(FIXTURE, bracketed) + system = dpdata.LabeledSystem(bracketed, fmt="cp2k/aimd_output") + self.assertGreater(system.get_nframes(), 0) + def test_missing_trajectory_names_the_pattern(self): self._remove("DPGEN-pos-1.xyz") with self.assertRaises(FileNotFoundError) as caught: @@ -61,7 +67,7 @@ def test_missing_directory_says_so(self): absent = os.path.join(self.tmp_dir, "absent") with self.assertRaises(FileNotFoundError) as caught: dpdata.LabeledSystem(absent, fmt="cp2k/aimd_output") - self.assertIn("is not a directory", str(caught.exception)) + self.assertIn("does not exist", str(caught.exception)) if __name__ == "__main__":