From e346ac33010dfa90853fa8720f5c75b67a136c7d Mon Sep 17 00:00:00 2001
From: "Josef M. Gallmetzer" <64498081+galjos@users.noreply.github.com>
Date: Mon, 24 Aug 2026 09:21:11 +0200
Subject: [PATCH] fix: parse xyz2gen --periodic values from the command line

The --periodic option declared choices=[True, False, None] without a
type converter, so argparse compared the raw command-line string
against Python objects and rejected every value, including the ones
its own error message suggested. The option could therefore never
hold anything but its None default. A converter now maps the strings
True, False and None (case-insensitive) to the corresponding Python
values and reports a proper error for anything else.
---
 PQAnalysis/cli/xyz2gen.py | 32 +++++++++++++++++
 tests/cli/test_xyz2gen.py | 76 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 108 insertions(+)
 create mode 100644 tests/cli/test_xyz2gen.py

diff --git a/PQAnalysis/cli/xyz2gen.py b/PQAnalysis/cli/xyz2gen.py
index 768efb60..061ab9d0 100644
--- a/PQAnalysis/cli/xyz2gen.py
+++ b/PQAnalysis/cli/xyz2gen.py
@@ -7,6 +7,8 @@
 
 """
 
+import argparse
+
 from PQAnalysis.config import code_base_url
 from PQAnalysis.io import xyz2gen
 from ._argument_parser import _ArgumentParser
@@ -27,6 +29,35 @@
 
 
 
+def _periodic(string: str) -> bool | None:
+    """
+    Converts a command line string to the periodic value.
+
+    Parameters
+    ----------
+    string : str
+        The command line string to be converted.
+
+    Returns
+    -------
+    bool | None
+        True for 'True', False for 'False' and None for 'None'
+        (case-insensitive).
+
+    Raises
+    ------
+    argparse.ArgumentTypeError
+        If the string is not one of 'True', 'False' or 'None'.
+    """
+    try:
+        return {'true': True, 'false': False, 'none': None}[string.lower()]
+    except KeyError:
+        raise argparse.ArgumentTypeError(
+            f"invalid choice: '{string}' (choose from True, False, None)"
+        ) from None
+
+
+
 class XYZ2GENCLI(CLIBase):
 
     """
@@ -65,6 +96,7 @@ def add_arguments(cls, parser: _ArgumentParser) -> None:
 
         parser.add_argument(
             '--periodic',
+            type=_periodic,
             choices=[True, False, None],
             default=None,
             help=(
diff --git a/tests/cli/test_xyz2gen.py b/tests/cli/test_xyz2gen.py
new file mode 100644
index 00000000..38c91939
--- /dev/null
+++ b/tests/cli/test_xyz2gen.py
@@ -0,0 +1,76 @@
+import pytest
+
+from unittest import mock
+
+from PQAnalysis.cli.xyz2gen import main, XYZ2GENCLI
+from PQAnalysis.cli._argument_parser import _ArgumentParser
+
+
+
+def test_name():
+    assert XYZ2GENCLI.program_name() == "xyz2gen"
+
+
+
+@pytest.mark.parametrize(
+    "value, expected",
+    [
+    ("True", True),
+    ("False", False),
+    ("None", None),
+    ("true", True),
+    ("false", False),
+    ("none", None),
+    ]
+)
+def test_periodic_argument_parsing(value, expected):
+    args = _parse(["md-01.xyz", "--periodic", value])
+
+    assert args.periodic is expected
+
+
+
+def test_periodic_argument_default_and_invalid(capsys):
+    args = _parse(["md-01.xyz"])
+    assert args.periodic is None
+
+    with pytest.raises(SystemExit):
+        _parse(["md-01.xyz", "--periodic", "maybe"])
+
+    captured = capsys.readouterr()
+    assert "invalid choice: 'maybe'" in captured.err
+
+
+
+def _parse(argv):
+    parser = _ArgumentParser(description="test")
+    XYZ2GENCLI.add_arguments(parser)
+    return parser.parse_args(argv)
+
+
+
+@pytest.mark.parametrize("example_dir", ["xyz2rst"], indirect=False)
+def test_main_periodic(test_with_data_dir):
+    with mock.patch(
+        "sys.argv",
+        ["xyz2gen", "md-01.xyz", "--periodic", "True", "-o", "box.gen"],
+    ):
+        main()
+
+    with open("box.gen", "r", encoding="utf-8") as file:
+        box_lines = file.read().splitlines()
+
+    assert box_lines[0].split() == ["4", "S"]
+    assert len(box_lines) == 10  # header + elements + 4 atoms + 4 cell lines
+
+    with mock.patch(
+        "sys.argv",
+        ["xyz2gen", "md-01.xyz", "--periodic", "False", "-o", "nobox.gen"],
+    ):
+        main()
+
+    with open("nobox.gen", "r", encoding="utf-8") as file:
+        nobox_lines = file.read().splitlines()
+
+    assert nobox_lines[0].split() == ["4", "C"]
+    assert len(nobox_lines) == 6  # header + elements + 4 atoms, no cell