diff --git a/alphapulldown/folding_backend/alphafold2_backend.py b/alphapulldown/folding_backend/alphafold2_backend.py index e4b6c498..d20f9598 100644 --- a/alphapulldown/folding_backend/alphafold2_backend.py +++ b/alphapulldown/folding_backend/alphafold2_backend.py @@ -10,6 +10,7 @@ import json import pickle import subprocess +import sys import enum import typing from collections.abc import Mapping @@ -602,6 +603,41 @@ def _record_inference_kernels(output_dir, model_name, model_runner) -> None: json.dump(existing, f, indent=2, sort_keys=True) +# The converter runs in its own process: it exits on errors and keeps module-level state. +_MODELCIF_CONVERTER = os.path.join( + os.path.dirname(os.path.dirname(os.path.abspath(__file__))), + "scripts", + "convert_to_modelcif.py", +) + + +def _convert_to_modelcif(output_dir) -> None: + """Convert the fold's models to ModelCIF. Best effort: a failure is reported, never + raised, so the fold is still cleaned up and recorded complete. + + The converter runs with this interpreter and an argument list, not a shell, so any + output path works; what it prints is passed on as diagnostics, and only its exit + code says whether it failed. + """ + command = [sys.executable, _MODELCIF_CONVERTER, "--ap_output", os.fspath(output_dir)] + logging.info(f"Converting {output_dir} to ModelCIF format...") + try: + result = subprocess.run(command, check=False, capture_output=True, text=True) + except OSError as exc: + logging.warning(f"Could not start the ModelCIF conversion of {output_dir}: {exc}") + return + for stream, text in (("stdout", result.stdout), ("stderr", result.stderr)): + if text and text.strip(): + logging.info(f"ModelCIF converter {stream}:\n{text.rstrip()}") + if result.returncode != 0: + logging.warning( + f"ModelCIF conversion of {output_dir} failed with exit code " + f"{result.returncode}; its output is logged above." + ) + else: + logging.info(f"ModelCIF converter finished for {output_dir}.") + + class AlphaFold2Backend(FoldingBackend): """ A backend to perform structure prediction using AlphaFold. @@ -1302,23 +1338,8 @@ def postprocess( # template_file_path, ranked_output_path, temp_dir # ) - # Call convert_to_modelcif script if convert_to_modelcif: - parent_dir = os.path.dirname(os.path.dirname((os.path.abspath(__file__)))) - logging.info(f"Converting {output_dir} to ModelCIF format...") - command = f"python3 {parent_dir}/scripts/convert_to_modelcif.py " \ - f"--ap_output {output_dir} " - - result = subprocess.run(command, - check=False, - shell=True, - capture_output=True, - text=True) - - if result.stderr: - logging.error(f"Error: {result.stderr}") - else: - logging.info("All PDBs converted to ModelCIF format.") + _convert_to_modelcif(output_dir) post_prediction_process( output_dir, compress_pickles=compress_pickles, diff --git a/test/integration/test_modelcif.py b/test/integration/test_modelcif.py index 5e1a55d8..5e57d4ab 100644 --- a/test/integration/test_modelcif.py +++ b/test/integration/test_modelcif.py @@ -159,3 +159,41 @@ def test_missing_fasta_falls_back_to_structure_sequence(self): with open(out_cif, "r", encoding="ascii") as fh: cif_txt = fh.read() self.assertIn("MESAIA", cif_txt) + + def test_backend_conversion_matches_the_script_in_any_output_path(self): + """The AF2 backend's conversion, run in a directory full of shell metacharacters, + writes what the standalone script writes in a plain one.""" + try: + from alphapulldown.folding_backend.alphafold2_backend import _convert_to_modelcif + except ImportError as exc: # the AF2 backend needs AlphaFold 2 + self.skipTest(f"AlphaFold 2 backend unavailable: {exc}") + with tempfile.TemporaryDirectory() as temp_dir: + # The fold directory's name becomes the ModelCIF entry ID, so both are TEST. + plain = Path(temp_dir, "plain", "TEST") + odd = Path(temp_dir, "runs A+B; touch pwned $(x) 'q' & `y`", "TEST") + for fold_dir in (plain, odd): + shutil.copytree(join(self.input_dir, "TEST"), fold_dir) + + subprocess.run( + [sys.executable, self.script_path, "--ap_output", str(plain)], + check=True, capture_output=True, text=True, + ) + _convert_to_modelcif(odd) + + files = sorted(path.relative_to(plain) for path in plain.rglob("*") if path.is_file()) + self.assertEqual( + files, sorted(path.relative_to(odd) for path in odd.rglob("*") if path.is_file()) + ) + self.assertIn(Path("ranked_0.cif"), files) + for relative in files: + if relative.suffix == ".zip": # archives differ in their timestamps only + with zipfile.ZipFile(plain / relative) as expected, zipfile.ZipFile( + odd / relative + ) as actual: + self.assertEqual(expected.namelist(), actual.namelist()) + for name in expected.namelist(): + self.assertEqual(expected.read(name), actual.read(name), name) + else: + self.assertEqual( + (plain / relative).read_bytes(), (odd / relative).read_bytes(), relative + ) diff --git a/test/unit/test_alphafold2_backend_helpers.py b/test/unit/test_alphafold2_backend_helpers.py index 892b2015..0a73e907 100644 --- a/test/unit/test_alphafold2_backend_helpers.py +++ b/test/unit/test_alphafold2_backend_helpers.py @@ -1248,7 +1248,15 @@ def test_postprocess_ranks_models_relaxes_best_and_runs_cleanup( assert (tmp_path / "ranked_1.pdb").read_text(encoding="utf-8") == "PDB:low" assert (tmp_path / "relax_metrics.json").is_file() assert cleanup_calls - assert subprocess_calls + (command,), kwargs = subprocess_calls[0] + assert command == [ + af2_backend_module.sys.executable, + af2_backend_module._MODELCIF_CONVERTER, + "--ap_output", + str(tmp_path), + ] + assert command[1].endswith("scripts/convert_to_modelcif.py") + assert "shell" not in kwargs def test_postprocess_relaxes_all_using_saved_unrelaxed_pdbs( @@ -1340,8 +1348,8 @@ def test_postprocess_handles_monomers_without_relaxation_and_logs_modelcif_error ) monkeypatch.setattr( af2_backend_module.logging, - "error", - lambda message: modelcif_errors.append(message), + "warning", + lambda message, *args: modelcif_errors.append(message % args if args else message), ) monkeypatch.setattr( af2_backend_module.subprocess, @@ -1383,7 +1391,9 @@ def test_postprocess_handles_monomers_without_relaxation_and_logs_modelcif_error assert not (tmp_path / "relaxed_modelA.pdb").exists() assert plot_calls == [0] assert cleanup_calls - assert modelcif_errors == ["Error: convert failed"] + assert [message for message in modelcif_errors if "ModelCIF" in message] == [ + f"ModelCIF conversion of {tmp_path} failed with exit code 1; its output is logged above." + ] def test_postprocess_skips_best_relaxation_below_score_threshold( @@ -2116,3 +2126,98 @@ def test_an_unreadable_json_left_by_an_older_version_is_ignored( assert af2_backend_module._read_from_json_if_exists(str(path)) == {} assert "Ignoring unreadable" in caplog.text + + +_FAKE_MODELCIF_CONVERTER = """ +import argparse, pathlib, sys +parser = argparse.ArgumentParser() +parser.add_argument("--ap_output") +output = pathlib.Path(parser.parse_args().ap_output) +(output / "ranked_0.cif").write_text("data_converted\\n") +print("INFO converting ranked_0.pdb", file=sys.stderr) +""" + + +def test_modelcif_conversion_works_in_any_output_path( + af2_backend_module, monkeypatch, tmp_path, caplog +): + converter = tmp_path / "convert_to_modelcif.py" + converter.write_text(_FAKE_MODELCIF_CONVERTER) + monkeypatch.setattr(af2_backend_module, "_MODELCIF_CONVERTER", str(converter)) + monkeypatch.chdir(tmp_path) + output_dir = tmp_path / "fold A+B; touch pwned $(touch pwned2) 'q' & `x`" + output_dir.mkdir() + caplog.set_level("INFO") + + af2_backend_module._convert_to_modelcif(output_dir) + + assert (output_dir / "ranked_0.cif").read_text() == "data_converted\n" + # No shell ran the command, so nothing else appeared. + assert sorted(path.name for path in tmp_path.iterdir()) == sorted( + [converter.name, output_dir.name] + ) + # Informational stderr is passed on, not taken for a failure. + assert "INFO converting ranked_0.pdb" in caplog.text + assert "failed" not in caplog.text + + +def _postprocess_monomer(af2_backend_module, monkeypatch, tmp_path, *, convert_to_modelcif): + cleanup_calls = [] + monkeypatch.setattr(af2_backend_module, "plot_pae_from_matrix", lambda **kwargs: None) + monkeypatch.setattr( + af2_backend_module, + "post_prediction_process", + lambda *args, **kwargs: cleanup_calls.append(args), + ) + af2_backend_module.start_fold_record(tmp_path, {"backend": "alphafold2"}) + af2_backend_module.AlphaFold2Backend.postprocess( + prediction_results={ + "modelA": { + "plddt": np.array([81.0, 82.0], dtype=np.float32), + "predicted_aligned_error": np.zeros((2, 2), dtype=np.float32), + "max_predicted_aligned_error": 31.0, + "ranking_confidence": 81.5, + "ptm": 0.4, + "seqs": ["AB"], + "unrelaxed_protein": SimpleNamespace(name="mono"), + } + }, + multimeric_object=af2_backend_module.MonomericObject("single", "AB"), + output_dir=tmp_path, + features_directory="/features", + models_to_relax=af2_backend_module.ModelsToRelax.NONE, + convert_to_modelcif=convert_to_modelcif, + ) + return cleanup_calls + + +@pytest.mark.parametrize("failure", ["nonzero exit", "launch error"]) +def test_a_failed_modelcif_conversion_still_cleans_up_and_completes_the_fold( + af2_backend_module, monkeypatch, tmp_path, caplog, failure +): + def run(command, **kwargs): + if failure == "launch error": + raise FileNotFoundError(command[0]) + return SimpleNamespace(returncode=2, stdout="", stderr="Traceback: ValueError") + + monkeypatch.setattr(af2_backend_module.subprocess, "run", run) + + cleanup_calls = _postprocess_monomer( + af2_backend_module, monkeypatch, tmp_path, convert_to_modelcif=True + ) + + assert cleanup_calls + assert af2_backend_module.read_fold_record(tmp_path)["complete"] is True + expected = "failed with exit code 2" if failure == "nonzero exit" else "Could not start" + assert expected in caplog.text + + +def test_no_modelcif_conversion_starts_when_it_is_off( + af2_backend_module, monkeypatch, tmp_path +): + def run(*args, **kwargs): + raise AssertionError("the ModelCIF converter started") + + monkeypatch.setattr(af2_backend_module.subprocess, "run", run) + + assert _postprocess_monomer(af2_backend_module, monkeypatch, tmp_path, convert_to_modelcif=False)