Replace procrunner use with subprocess (xia2#684)

Given procrunner was intended primarily to provide backwards compatibility to
legacy Python versions (which we no longer support), it makes sense to use the
standard Python library subprocess.run() method in its place.

conftest.py

@@ -9,9 +9,9 @@
 import argparse
 import os
 import re
+import subprocess
 from pathlib import Path
 
-import procrunner
 import pytest
 
 
@@ -50,7 +50,7 @@ def ccp4():
         pytest.skip("CCP4 installation required for this test")
 
     try:
-        result = procrunner.run(["refmac5", "-i"], print_stdout=False)
+        result = subprocess.run(["refmac5", "-i"], capture_output=True)
     except OSError:
         pytest.skip(
             "CCP4 installation required for this test - Could not find CCP4 executable"
@@ -74,7 +74,7 @@ def xds():
     Skip the test if XDS is not installed.
     """
     try:
-        result = procrunner.run(["xds"], print_stdout=False)
+        result = subprocess.run(["xds"], capture_output=True)
     except OSError:
         pytest.skip("XDS installation required for this test")
     version = re.search(rb"BUILT=([0-9]+)\)", result.stdout)
@@ -100,7 +100,7 @@ def ensure_repository_is_clean():
     if not os.getenv("CHECK_CLEAN_WORKDIR"):
         return
     print("Working directory:", _repository)
-    status = procrunner.run(("git", "status", "-s"), working_directory=_repository)
+    status = subprocess.run(("git", "status", "-s"), cwd=_repository)
     if status.stdout:
         assert False, "Working directory is not clean"
 

newsfragments/684.misc

@@ -0,0 +1 @@
+Replace ``procrunner.run()`` usage with standard Python library routine ``subprocess.run()``

setup.cfg

@@ -29,7 +29,6 @@ include_package_data = True
 install_requires =
     dials-data>=2.0
     Jinja2
-    procrunner
     pyyaml
     tabulate
 packages = find:

src/xia2/Handlers/Versions.py

@@ -5,8 +5,6 @@
 import subprocess
 from collections.abc import Mapping
 
-import procrunner
-
 import dials.util.version
 
 import xia2.XIA2Version
@@ -45,8 +43,10 @@ def get_xia2_version():
 @functools.lru_cache(maxsize=None)
 def get_xds_version():
     try:
-        result = procrunner.run(
-            ["xds"], print_stdout=False, print_stderr=False, stdin=subprocess.DEVNULL
+        result = subprocess.run(
+            ["xds"],
+            stdin=subprocess.DEVNULL,
+            capture_output=True,
         )
     except OSError:
         pass
@@ -58,11 +58,10 @@ def get_xds_version():
 
 @functools.lru_cache(maxsize=None)
 def get_aimless_version():
-    result = procrunner.run(
+    result = subprocess.run(
         ["aimless", "--no-input"],
-        print_stdout=False,
-        print_stderr=False,
         stdin=subprocess.DEVNULL,
+        capture_output=True,
     )
     version = re.search(rb"version\s\d+\.\d+\.\d+", result.stdout)
     if version:
@@ -72,8 +71,10 @@ def get_aimless_version():
 
 @functools.lru_cache(maxsize=None)
 def get_pointless_version():
-    result = procrunner.run(
-        ["pointless"], print_stdout=False, print_stderr=False, stdin=subprocess.DEVNULL
+    result = subprocess.run(
+        ["pointless"],
+        stdin=subprocess.DEVNULL,
+        capture_output=True,
     )
     version = re.search(rb"version\s\d+\.\d+\.\d+", result.stdout)
     if version:

src/xia2/Modules/SSX/data_integration_standard.py

@@ -6,11 +6,11 @@
 import math
 import os
 import pathlib
+import subprocess
 from dataclasses import asdict, dataclass, field
 from typing import List, Optional, Tuple
 
 import numpy as np
-import procrunner
 
 from dials.util.mp import multi_node_parallel_map
 from dxtbx.serialize import load
@@ -278,7 +278,7 @@ def run_import(working_directory: pathlib.Path, file_input: FileInput) -> None:
     else:
         xia2_logger.notice(banner("Importing"))  # type: ignore
     with record_step("dials.import"):
-        result = procrunner.run(import_command, working_directory=working_directory)
+        result = subprocess.run(import_command, cwd=working_directory)
         if result.returncode or result.stderr:
             raise ValueError(
                 "dials.import returned error status:\n" + str(result.stderr)

src/xia2/cli/make_sphinx_html.py

@@ -4,8 +4,6 @@
 import shutil
 import subprocess
 
-import procrunner
-
 import xia2
 
 
@@ -18,15 +16,11 @@ def run():
     sphinx_dir = xia2_dir / "doc" / "sphinx"
     if dest_dir.is_dir():
         shutil.rmtree(dest_dir)
-    result = procrunner.run(
-        ["make", "clean"], working_directory=sphinx_dir, stdin=subprocess.DEVNULL
-    )
+    result = subprocess.run(["make", "clean"], cwd=sphinx_dir, stdin=subprocess.DEVNULL)
     if result.returncode:
         exit(f"make clean failed with exit code {result.returncode}")
 
-    result = procrunner.run(
-        ["make", "html"], working_directory=sphinx_dir, stdin=subprocess.DEVNULL
-    )
+    result = subprocess.run(["make", "html"], cwd=sphinx_dir, stdin=subprocess.DEVNULL)
     if result.returncode:
         exit(f"make html failed with exit code {result.returncode}")
 

tests/Applications/test_xia2setup.py

@@ -1,6 +1,8 @@
 from __future__ import annotations
 
-import procrunner
+import os
+import subprocess
+
 import pytest
 
 from xia2.Handlers.XInfo import XInfo
@@ -18,13 +20,13 @@ def insulin_with_missing_image(dials_data, tmp_path):
 
 
 def test_write_xinfo_insulin_with_missing_image(insulin_with_missing_image, tmp_path):
-    result = procrunner.run(
+    result = subprocess.run(
         [
             "xia2.setup",
             f"image={insulin_with_missing_image.parent.joinpath('insulin_1_001.img')}",
         ],
-        environment_override={"CCP4": tmp_path},
-        working_directory=tmp_path,
+        env={"CCP4": tmp_path, **os.environ},
+        cwd=tmp_path,
     )
     assert not result.returncode
     assert not result.stderr
@@ -37,14 +39,14 @@ def test_write_xinfo_insulin_with_missing_image(insulin_with_missing_image, tmp_
 
 
 def test_write_xinfo_template_missing_images(insulin_with_missing_image, tmp_path):
-    result = procrunner.run(
+    result = subprocess.run(
         [
             "xia2.setup",
             f"image={insulin_with_missing_image.parent.joinpath('insulin_1_001.img:1:22')}",
             "read_all_image_headers=False",
         ],
-        environment_override={"CCP4": tmp_path},
-        working_directory=tmp_path,
+        env={"CCP4": tmp_path, **os.environ},
+        cwd=tmp_path,
     )
     assert not result.returncode
     assert not result.stderr
@@ -56,15 +58,15 @@ def test_write_xinfo_template_missing_images(insulin_with_missing_image, tmp_pat
 
 
 def test_write_xinfo_split_sweep(dials_data, tmp_path):
-    result = procrunner.run(
+    result = subprocess.run(
         [
             "xia2.setup",
             f"image={dials_data('insulin', pathlib=True) / 'insulin_1_001.img:1:22'}",
             f"image={dials_data('insulin', pathlib=True) / 'insulin_1_001.img:23:45'}",
             "read_all_image_headers=False",
         ],
-        environment_override={"CCP4": tmp_path},
-        working_directory=tmp_path,
+        env={"CCP4": tmp_path, **os.environ},
+        cwd=tmp_path,
     )
     assert not result.returncode
     assert not result.stderr
@@ -78,14 +80,14 @@ def test_write_xinfo_split_sweep(dials_data, tmp_path):
 
 def test_write_xinfo_unroll(dials_data, tmp_path):
     # This test partially exercises the fix to https://github.com/xia2/xia2/issues/498 with a different syntax
-    result = procrunner.run(
+    result = subprocess.run(
         [
             "xia2.setup",
             f"image={dials_data('insulin', pathlib=True) / 'insulin_1_001.img:1:45:15'}",
             "read_all_image_headers=False",
         ],
-        environment_override={"CCP4": tmp_path},
-        working_directory=tmp_path,
+        env={"CCP4": tmp_path, **os.environ},
+        cwd=tmp_path,
     )
     assert not result.returncode
     assert not result.stderr

tests/regression/test_X4_wide.py

@@ -1,6 +1,7 @@
 from __future__ import annotations
 
-import procrunner
+import subprocess
+
 import pytest
 
 import iotbx.mtz
@@ -46,9 +47,10 @@ def _split_xinfo(data_dir, tmp_path) -> str:
 
 @pytest.mark.parametrize("pipeline,scaler", (("dials", "xdsa"), ("3dii", "dials")))
 def test_incompatible_pipeline_scaler(pipeline, scaler, tmp_path, ccp4):
-    result = procrunner.run(
+    result = subprocess.run(
         ["xia2", f"pipeline={pipeline}", "nproc=1", f"scaler={scaler}"],
-        working_directory=tmp_path,
+        cwd=tmp_path,
+        capture_output=True,
     )
     assert result.returncode
     assert (
@@ -67,7 +69,7 @@ def test_dials_aimless(regression_test, dials_data, tmp_path, ccp4):
         "truncate=cctbx",
         dials_data("x4wide", pathlib=True),
     ]
-    result = procrunner.run(command_line, working_directory=tmp_path)
+    result = subprocess.run(command_line, cwd=tmp_path, capture_output=True)
     success, issues = xia2.Test.regression.check_result(
         "X4_wide.dials-aimless", result, tmp_path, ccp4, expected_space_group="P41212"
     )
@@ -86,7 +88,7 @@ def test_dials_aimless_with_dials_pipeline(regression_test, dials_data, tmp_path
         "truncate=cctbx",
         dials_data("x4wide", pathlib=True),
     ]
-    result = procrunner.run(command_line, working_directory=tmp_path)
+    result = subprocess.run(command_line, cwd=tmp_path, capture_output=True)
     success, issues = xia2.Test.regression.check_result(
         "X4_wide.dials-aimless", result, tmp_path, ccp4
     )
@@ -106,7 +108,7 @@ def test_dials(regression_test, dials_data, tmp_path, ccp4):
         "crystal=bar",
         dials_data("x4wide", pathlib=True),
     ]
-    result = procrunner.run(command_line, working_directory=tmp_path)
+    result = subprocess.run(command_line, cwd=tmp_path, capture_output=True)
     scaled_expt_file = tmp_path / "DataFiles" / "foo_bar_scaled.expt"
     assert scaled_expt_file.is_file()
     scaled_expt = load.experiment_list(scaled_expt_file)
@@ -153,7 +155,7 @@ def test_dials_aimless_split(regression_test, dials_data, tmp_path, ccp4):
         "trust_beam_centre=True",
         "xinfo=%s" % _split_xinfo(dials_data("x4wide", pathlib=True), tmp_path),
     ]
-    result = procrunner.run(command_line, working_directory=tmp_path)
+    result = subprocess.run(command_line, cwd=tmp_path, capture_output=True)
     success, issues = xia2.Test.regression.check_result(
         "X4_wide_split.dials-aimless", result, tmp_path, ccp4
     )
@@ -170,7 +172,7 @@ def test_dials_split(regression_test, dials_data, tmp_path, ccp4):
         "xinfo=%s" % _split_xinfo(dials_data("x4wide", pathlib=True), tmp_path),
         "mode=parallel",
     ]
-    result = procrunner.run(command_line, working_directory=tmp_path)
+    result = subprocess.run(command_line, cwd=tmp_path, capture_output=True)
     success, issues = xia2.Test.regression.check_result(
         "X4_wide_split.dials",
         result,
@@ -193,7 +195,7 @@ def test_xds(regression_test, dials_data, tmp_path, ccp4, xds):
         "read_all_image_headers=False",
         dials_data("x4wide", pathlib=True),
     ]
-    result = procrunner.run(command_line, working_directory=tmp_path)
+    result = subprocess.run(command_line, cwd=tmp_path, capture_output=True)
     success, issues = xia2.Test.regression.check_result(
         "X4_wide.xds", result, tmp_path, ccp4, xds, expected_space_group="P41212"
     )
@@ -210,7 +212,7 @@ def test_xds_split(regression_test, dials_data, tmp_path, ccp4, xds):
         "trust_beam_centre=True",
         "xinfo=%s" % _split_xinfo(dials_data("x4wide", pathlib=True), tmp_path),
     ]
-    result = procrunner.run(command_line, working_directory=tmp_path)
+    result = subprocess.run(command_line, cwd=tmp_path, capture_output=True)
     success, issues = xia2.Test.regression.check_result(
         "X4_wide_split.xds", result, tmp_path, ccp4, xds
     )
@@ -226,7 +228,7 @@ def test_xds_ccp4a(regression_test, dials_data, tmp_path, ccp4, xds):
         "trust_beam_centre=True",
         dials_data("x4wide", pathlib=True),
     ]
-    result = procrunner.run(command_line, working_directory=tmp_path)
+    result = subprocess.run(command_line, cwd=tmp_path, capture_output=True)
     success, issues = xia2.Test.regression.check_result(
         "X4_wide.ccp4a", result, tmp_path, ccp4, xds
     )
@@ -245,7 +247,7 @@ def test_xds_ccp4a_split(regression_test, dials_data, tmp_path, ccp4, xds):
         "mode=parallel",
         "xinfo=%s" % _split_xinfo(dials_data("x4wide", pathlib=True), tmp_path),
     ]
-    result = procrunner.run(command_line, working_directory=tmp_path)
+    result = subprocess.run(command_line, cwd=tmp_path, capture_output=True)
     success, issues = xia2.Test.regression.check_result(
         "X4_wide_split.ccp4a", result, tmp_path, ccp4, xds
     )
@@ -269,7 +271,7 @@ def test_space_group_dials(
         "image=%s"
         % dials_data("x4wide", pathlib=True).joinpath("X4_wide_M1S4_2_0001.cbf:20:30"),
     ]
-    result = procrunner.run(command_line, working_directory=tmp_path)
+    result = subprocess.run(command_line, cwd=tmp_path, capture_output=True)
     success, issues = xia2.Test.regression.check_result(
         "X4_wide.space_group.%s" % pipeline,
         result,
@@ -296,7 +298,7 @@ def test_space_group_3dii(
         "image=%s"
         % dials_data("x4wide", pathlib=True).joinpath("X4_wide_M1S4_2_0001.cbf:20:30"),
     ]
-    result = procrunner.run(command_line, working_directory=tmp_path)
+    result = subprocess.run(command_line, cwd=tmp_path, capture_output=True)
     success, issues = xia2.Test.regression.check_result(
         "X4_wide.space_group.3dii",
         result,

tests/regression/test_mad_example.py

@@ -1,6 +1,6 @@
 from __future__ import annotations
 
-import procrunner
+import subprocess
 
 import xia2.Test.regression
 
@@ -25,7 +25,7 @@ def test_dials(regression_test, dials_data, tmp_path, ccp4):
         "trust_beam_centre=True",
         dials_data("fumarase", pathlib=True),
     ]
-    result = procrunner.run(command_line, working_directory=tmp_path)
+    result = subprocess.run(command_line, cwd=tmp_path)
     success, issues = xia2.Test.regression.check_result(
         "mad_example.dials",
         result,
@@ -46,7 +46,7 @@ def test_dials_aimless(regression_test, dials_data, tmp_path, ccp4):
         "trust_beam_centre=True",
         dials_data("fumarase", pathlib=True),
     ]
-    result = procrunner.run(command_line, working_directory=tmp_path)
+    result = subprocess.run(command_line, cwd=tmp_path)
     success, issues = xia2.Test.regression.check_result(
         "mad_example.dials-aimless",
         result,
@@ -67,7 +67,7 @@ def test_xds(regression_test, dials_data, tmp_path, ccp4, xds):
         "trust_beam_centre=True",
         dials_data("fumarase", pathlib=True),
     ]
-    result = procrunner.run(command_line, working_directory=tmp_path)
+    result = subprocess.run(command_line, cwd=tmp_path)
     success, issues = xia2.Test.regression.check_result(
         "mad_example.xds",
         result,
@@ -90,7 +90,7 @@ def test_xds_ccp4a(regression_test, dials_data, tmp_path, ccp4, xds):
         "scaler=ccp4a",
         dials_data("fumarase", pathlib=True),
     ]
-    result = procrunner.run(command_line, working_directory=tmp_path)
+    result = subprocess.run(command_line, cwd=tmp_path)
     success, issues = xia2.Test.regression.check_result(
         "mad_example.ccp4a",
         result,