Merge pull request #374 from tovrstra/cli-allow-changes

Add command-line option `--allow-changes` to `iodata-convert`

Author: tovrstra

iodata/__main__.py

@@ -20,6 +20,7 @@
 """CLI for file conversion."""
 
 import argparse
+from typing import Optional
 
 import numpy as np
 
@@ -31,7 +32,7 @@
     __version__ = "0.0.0.post0"
 
 
-__all__ = ()
+__all__ = ("convert",)
 
 
 DESCRIPTION = """\
@@ -83,6 +84,14 @@ def parse_args():
     parser.add_argument(
         "-o", "--outfmt", help="Select the output format, overrides automatic detection."
     )
+    parser.add_argument(
+        "-c",
+        "--allow-changes",
+        default=False,
+        action="store_true",
+        help="Allow (not trivially reversible) conversion of the input data to make it compatible "
+        "with the output format. Warnings will be emitted for all changes made.",
+    )
     parser.add_argument(
         "-m",
         "--many",
@@ -95,7 +104,14 @@ def parse_args():
     return parser.parse_args()
 
 
-def convert(infn, outfn, many, infmt, outfmt):
+def convert(
+    infn: str,
+    outfn: str,
+    many: bool = False,
+    infmt: Optional[str] = None,
+    outfmt: Optional[str] = None,
+    allow_changes: bool = False,
+):
     """Convert file from one format to another.
 
     Parameters
@@ -110,12 +126,15 @@ def convert(infn, outfn, many, infmt, outfmt):
         The input format.
     outfmt
         The output format.
+    allow_changes
+        Allow prepare_dump functions to modify the data
+        to make it compatible with the output format.
 
     """
     if many:
-        dump_many(load_many(infn, fmt=infmt), outfn, fmt=outfmt)
+        dump_many(load_many(infn, fmt=infmt), outfn, allow_changes=allow_changes, fmt=outfmt)
     else:
-        dump_one(load_one(infn, fmt=infmt), outfn, fmt=outfmt)
+        dump_one(load_one(infn, fmt=infmt), outfn, allow_changes=allow_changes, fmt=outfmt)
 
 
 def main():
@@ -124,7 +143,7 @@ def main():
     np.seterr(divide="raise", over="raise", invalid="raise")
 
     args = parse_args()
-    convert(args.input, args.output, args.many, args.infmt, args.outfmt)
+    convert(args.input, args.output, args.many, args.infmt, args.outfmt, args.allow_changes)
 
 
 if __name__ == "__main__":

iodata/test/test_cli.py

@@ -18,59 +18,118 @@
 # --
 """Unit tests for iodata.__main__."""
 
-import functools
 import os
 import subprocess
 import sys
+from functools import partial
 from importlib.resources import as_file, files
+from typing import Optional
+from warnings import warn
 
+import pytest
 from numpy.testing import assert_allclose, assert_equal
 
-from ..__main__ import convert
+from ..__main__ import convert as convfn
 from ..api import load_many, load_one
+from ..utils import FileFormatError, PrepareDumpError, PrepareDumpWarning
+
+
+def _convscript(
+    infn: str,
+    outfn: str,
+    many: bool = False,
+    infmt: Optional[str] = None,
+    outfmt: Optional[str] = None,
+    allow_changes: bool = False,
+):
+    """Simulate the convert function by calling iodata-convert in a subprocess."""
+    args = [sys.executable, "-m", "iodata.__main__", infn, outfn]
+    if many:
+        args.append("-m")
+    if infmt is not None:
+        args.append(f"--infmt={infmt}")
+    if outfmt is not None:
+        args.append(f"--outfmt={outfmt}")
+    if allow_changes:
+        args.append("-c")
+    cp = subprocess.run(args, capture_output=True, check=False, encoding="utf8")
+    if cp.returncode == 0:
+        if allow_changes and "PrepareDumpWarning" in cp.stderr:
+            warn(PrepareDumpWarning(cp.stderr), stacklevel=2)
+    else:
+        if "PrepareDumpError" in cp.stderr:
+            raise PrepareDumpError(cp.stderr)
+        if "FileFormatError" in cp.stderr:
+            raise FileFormatError(cp.stderr)
+        raise RuntimeError(f"Failure not processed.\n{cp.stderr}")
 
 
 def _check_convert_one(myconvert, tmpdir):
     outfn = os.path.join(tmpdir, "tmp.xyz")
     with as_file(files("iodata.test.data").joinpath("hf_sto3g.fchk")) as infn:
-        myconvert(infn, outfn)
+        myconvert(infn, outfn, allow_changes=False)
     iodata = load_one(outfn)
     assert iodata.natom == 2
     assert_equal(iodata.atnums, [9, 1])
     assert_allclose(iodata.atcoords, [[0.0, 0.0, 0.190484394], [0.0, 0.0, -1.71435955]])
 
 
-def test_convert_one_autofmt(tmpdir):
-    myconvert = functools.partial(convert, many=False, infmt=None, outfmt=None)
-    _check_convert_one(myconvert, tmpdir)
+def _check_convert_one_changes(myconvert, tmpdir):
+    outfn = os.path.join(tmpdir, "tmp.mkl")
+    with as_file(files("iodata.test.data").joinpath("hf_sto3g.fchk")) as infn:
+        with pytest.raises(PrepareDumpError):
+            myconvert(infn, outfn, allow_changes=False)
+        assert not os.path.isfile(outfn)
+        with pytest.warns(PrepareDumpWarning):
+            myconvert(infn, outfn, allow_changes=True)
+    iodata = load_one(outfn)
+    assert iodata.natom == 2
+    assert_equal(iodata.atnums, [9, 1])
+    assert_allclose(iodata.atcoords, [[0.0, 0.0, 0.190484394], [0.0, 0.0, -1.71435955]])
 
 
-def test_convert_one_manfmt(tmpdir):
-    myconvert = functools.partial(convert, many=False, infmt="fchk", outfmt="xyz")
+@pytest.mark.parametrize("convert", [convfn, _convscript])
+def test_convert_one_autofmt(tmpdir, convert):
+    myconvert = partial(convfn, many=False, infmt=None, outfmt=None)
     _check_convert_one(myconvert, tmpdir)
 
 
-def test_script_one_autofmt(tmpdir):
-    def myconvert(infn, outfn):
-        subprocess.run([sys.executable, "-m", "iodata.__main__", infn, outfn], check=True)
+@pytest.mark.parametrize("convert", [convfn, _convscript])
+def test_convert_one_autofmt_changes(tmpdir, convert):
+    myconvert = partial(convert, many=False, infmt=None, outfmt=None)
+    _check_convert_one_changes(myconvert, tmpdir)
+
 
+@pytest.mark.parametrize("convert", [convfn, _convscript])
+def test_convert_one_manfmt(tmpdir, convert):
+    myconvert = partial(convert, many=False, infmt="fchk", outfmt="xyz")
     _check_convert_one(myconvert, tmpdir)
 
 
-def test_script_one_manfmt(tmpdir):
-    def myconvert(infn, outfn):
-        subprocess.run(
-            [sys.executable, "-m", "iodata.__main__", infn, outfn, "-i", "fchk", "-o", "xyz"],
-            check=True,
-        )
+@pytest.mark.parametrize("convert", [convfn, _convscript])
+def test_convert_one_nonexisting_infmt(tmpdir, convert):
+    myconvert = partial(convert, many=False, infmt="blablabla", outfmt="xyz")
+    with pytest.raises(FileFormatError):
+        _check_convert_one(myconvert, tmpdir)
+
+
+@pytest.mark.parametrize("convert", [convfn, _convscript])
+def test_convert_one_nonexisting_outfmt(tmpdir, convert):
+    myconvert = partial(convert, many=False, infmt="fchk", outfmt="blablabla")
+    with pytest.raises(FileFormatError):
+        _check_convert_one(myconvert, tmpdir)
 
-    _check_convert_one(myconvert, tmpdir)
+
+@pytest.mark.parametrize("convert", [convfn, _convscript])
+def test_convert_one_manfmt_changes(tmpdir, convert):
+    myconvert = partial(convert, many=False, infmt="fchk", outfmt="molekel")
+    _check_convert_one_changes(myconvert, tmpdir)
 
 
 def _check_convert_many(myconvert, tmpdir):
     outfn = os.path.join(tmpdir, "tmp.xyz")
     with as_file(files("iodata.test.data").joinpath("peroxide_relaxed_scan.fchk")) as infn:
-        myconvert(infn, outfn)
+        myconvert(infn, outfn, allow_changes=False)
     trj = list(load_many(outfn))
     assert len(trj) == 13
     for iodata in trj:
@@ -80,28 +139,13 @@ def _check_convert_many(myconvert, tmpdir):
     assert_allclose(trj[5].atcoords[0], [0.0, 1.32466211, 0.0], atol=1e-5)
 
 
-def test_convert_many_autofmt(tmpdir):
-    myconvert = functools.partial(convert, many=True, infmt=None, outfmt=None)
+@pytest.mark.parametrize("convert", [convfn, _convscript])
+def test_convert_many_autofmt(tmpdir, convert):
+    myconvert = partial(convert, many=True, infmt=None, outfmt=None)
     _check_convert_many(myconvert, tmpdir)
 
 
-def test_convert_many_manfmt(tmpdir):
-    myconvert = functools.partial(convert, many=True, infmt="fchk", outfmt="xyz")
-    _check_convert_many(myconvert, tmpdir)
-
-
-def test_script_many_autofmt(tmpdir):
-    def myconvert(infn, outfn):
-        subprocess.run([sys.executable, "-m", "iodata.__main__", infn, outfn, "-m"], check=True)
-
-    _check_convert_many(myconvert, tmpdir)
-
-
-def test_script_many_manfmt(tmpdir):
-    def myconvert(infn, outfn):
-        subprocess.run(
-            [sys.executable, "-m", "iodata.__main__", infn, outfn, "-m", "-i", "fchk", "-o", "xyz"],
-            check=True,
-        )
-
+@pytest.mark.parametrize("convert", [convfn, _convscript])
+def test_convert_many_manfmt(tmpdir, convert):
+    myconvert = partial(convert, many=True, infmt="fchk", outfmt="xyz")
     _check_convert_many(myconvert, tmpdir)