From 17bad90dc4ef91b437c505fea238198c4c3b8fd5 Mon Sep 17 00:00:00 2001 From: Nathan Baker Date: Mon, 18 May 2020 20:37:56 -0700 Subject: [PATCH 1/5] Start fixing tests. --- Tests/pkacalc_test.py | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/Tests/pkacalc_test.py b/Tests/pkacalc_test.py index 8fd4e37..5a2f5ca 100644 --- a/Tests/pkacalc_test.py +++ b/Tests/pkacalc_test.py @@ -1,10 +1,12 @@ - +"""Reproduce tests from original PROPKA.""" import os import re from subprocess import call import sys import unittest import logging +import propka.lib +import propka.molecular_container # This error tolerance was chosen to make Ubuntu 18.04 pass under Windows @@ -12,6 +14,22 @@ import logging ACCEPTABLE_ERROR = 0.001 +def run_propka(args): + """Run PROPKA. + + Args: + args: command-line arguments for PROPKA + """ + options = propka.lib.loadOptions() + raise NotImplementedError("Need to incorporated command-line arguments") + pdbfiles = options.filenames + + for pdbfile in pdbfiles: + my_molecule = propka.molecular_container.Molecular_container(pdbfile, options) + my_molecule.calculate_pka() + my_molecule.write_pka() + + # Setting this up as a direct translation of the original runtest.py script # that will be run as part of 'python setup.py test'. This takes on the # order of 10s to run. From fa4a547f1f7af8be528f65a9969e676d2aa9f6e6 Mon Sep 17 00:00:00 2001 From: Nathan Baker Date: Thu, 21 May 2020 10:29:45 -0700 Subject: [PATCH 2/5] Enforce Python 3.*. --- setup.py | 1 + 1 file changed, 1 insertion(+) diff --git a/setup.py b/setup.py index b318fdd..f95aab5 100755 --- a/setup.py +++ b/setup.py @@ -56,5 +56,6 @@ See http://propka.org/ for the PROPKA web server. ], }, zip_safe=True, + python_requires='>=3', test_suite="Tests", ) From 393f5cee50601e89f9bc8439e851934e5738b9b0 Mon Sep 17 00:00:00 2001 From: Nathan Baker Date: Thu, 21 May 2020 10:39:08 -0700 Subject: [PATCH 3/5] Rename "Tests" directory for consistency. --- setup.py | 2 +- {Tests => tests}/.gitignore | 0 {Tests => tests}/__init__.py | 0 {Tests => tests}/hybrid36.py | 0 {Tests => tests}/pdb/1FTJ-Chain-A.pdb | 0 {Tests => tests}/pdb/1HPX-warn.pdb | 0 {Tests => tests}/pdb/1HPX.pdb | 0 {Tests => tests}/pdb/3SGB-subset.pdb | 0 {Tests => tests}/pdb/3SGB.pdb | 0 {Tests => tests}/pdb/4DFR.pdb | 0 {Tests => tests}/pkacalc_test.py | 0 {Tests => tests}/results/1FTJ-Chain-A.dat | 0 {Tests => tests}/results/1HPX-warn.dat | 0 {Tests => tests}/results/1HPX.dat | 0 {Tests => tests}/results/3SGB-subset.dat | 0 {Tests => tests}/results/3SGB.dat | 0 {Tests => tests}/results/4DFR.dat | 0 {Tests => tests}/runtest.py | 0 18 files changed, 1 insertion(+), 1 deletion(-) rename {Tests => tests}/.gitignore (100%) rename {Tests => tests}/__init__.py (100%) rename {Tests => tests}/hybrid36.py (100%) rename {Tests => tests}/pdb/1FTJ-Chain-A.pdb (100%) rename {Tests => tests}/pdb/1HPX-warn.pdb (100%) rename {Tests => tests}/pdb/1HPX.pdb (100%) rename {Tests => tests}/pdb/3SGB-subset.pdb (100%) rename {Tests => tests}/pdb/3SGB.pdb (100%) rename {Tests => tests}/pdb/4DFR.pdb (100%) rename {Tests => tests}/pkacalc_test.py (100%) rename {Tests => tests}/results/1FTJ-Chain-A.dat (100%) rename {Tests => tests}/results/1HPX-warn.dat (100%) rename {Tests => tests}/results/1HPX.dat (100%) rename {Tests => tests}/results/3SGB-subset.dat (100%) rename {Tests => tests}/results/3SGB.dat (100%) rename {Tests => tests}/results/4DFR.dat (100%) rename {Tests => tests}/runtest.py (100%) diff --git a/setup.py b/setup.py index f95aab5..a8363de 100755 --- a/setup.py +++ b/setup.py @@ -57,5 +57,5 @@ See http://propka.org/ for the PROPKA web server. }, zip_safe=True, python_requires='>=3', - test_suite="Tests", + test_suite="tests", ) diff --git a/Tests/.gitignore b/tests/.gitignore similarity index 100% rename from Tests/.gitignore rename to tests/.gitignore diff --git a/Tests/__init__.py b/tests/__init__.py similarity index 100% rename from Tests/__init__.py rename to tests/__init__.py diff --git a/Tests/hybrid36.py b/tests/hybrid36.py similarity index 100% rename from Tests/hybrid36.py rename to tests/hybrid36.py diff --git a/Tests/pdb/1FTJ-Chain-A.pdb b/tests/pdb/1FTJ-Chain-A.pdb similarity index 100% rename from Tests/pdb/1FTJ-Chain-A.pdb rename to tests/pdb/1FTJ-Chain-A.pdb diff --git a/Tests/pdb/1HPX-warn.pdb b/tests/pdb/1HPX-warn.pdb similarity index 100% rename from Tests/pdb/1HPX-warn.pdb rename to tests/pdb/1HPX-warn.pdb diff --git a/Tests/pdb/1HPX.pdb b/tests/pdb/1HPX.pdb similarity index 100% rename from Tests/pdb/1HPX.pdb rename to tests/pdb/1HPX.pdb diff --git a/Tests/pdb/3SGB-subset.pdb b/tests/pdb/3SGB-subset.pdb similarity index 100% rename from Tests/pdb/3SGB-subset.pdb rename to tests/pdb/3SGB-subset.pdb diff --git a/Tests/pdb/3SGB.pdb b/tests/pdb/3SGB.pdb similarity index 100% rename from Tests/pdb/3SGB.pdb rename to tests/pdb/3SGB.pdb diff --git a/Tests/pdb/4DFR.pdb b/tests/pdb/4DFR.pdb similarity index 100% rename from Tests/pdb/4DFR.pdb rename to tests/pdb/4DFR.pdb diff --git a/Tests/pkacalc_test.py b/tests/pkacalc_test.py similarity index 100% rename from Tests/pkacalc_test.py rename to tests/pkacalc_test.py diff --git a/Tests/results/1FTJ-Chain-A.dat b/tests/results/1FTJ-Chain-A.dat similarity index 100% rename from Tests/results/1FTJ-Chain-A.dat rename to tests/results/1FTJ-Chain-A.dat diff --git a/Tests/results/1HPX-warn.dat b/tests/results/1HPX-warn.dat similarity index 100% rename from Tests/results/1HPX-warn.dat rename to tests/results/1HPX-warn.dat diff --git a/Tests/results/1HPX.dat b/tests/results/1HPX.dat similarity index 100% rename from Tests/results/1HPX.dat rename to tests/results/1HPX.dat diff --git a/Tests/results/3SGB-subset.dat b/tests/results/3SGB-subset.dat similarity index 100% rename from Tests/results/3SGB-subset.dat rename to tests/results/3SGB-subset.dat diff --git a/Tests/results/3SGB.dat b/tests/results/3SGB.dat similarity index 100% rename from Tests/results/3SGB.dat rename to tests/results/3SGB.dat diff --git a/Tests/results/4DFR.dat b/tests/results/4DFR.dat similarity index 100% rename from Tests/results/4DFR.dat rename to tests/results/4DFR.dat diff --git a/Tests/runtest.py b/tests/runtest.py similarity index 100% rename from Tests/runtest.py rename to tests/runtest.py From fc27d8bfc3fc978d6a9a6b2dd644eb93e532f957 Mon Sep 17 00:00:00 2001 From: Nathan Baker Date: Thu, 21 May 2020 11:53:03 -0700 Subject: [PATCH 4/5] Fix bugs in invocation and options. Add non-command-arguments. Restore --quiet option. --- propka/lib.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/propka/lib.py b/propka/lib.py index 31b2880..fc2da5a 100644 --- a/propka/lib.py +++ b/propka/lib.py @@ -210,17 +210,21 @@ def build_parser(parser=None): "ligand bond orders")) group.add_argument("-k", "--keep-protons", dest="keep_protons", action="store_true", help="Keep protons in input file", default=False) + group.add_argument("-q", "--quiet", action="store_const", const="WARNING", + dest="log_level", help="supress non-warning messages") group.add_argument("--protonate-all", dest="protonate_all", action="store_true", help="Protonate all atoms (will not influence pKa calculation)", default=False) return parser -def loadOptions(*args): +def loadOptions(args): """ Load the arguments parser with options. Note that verbosity is set as soon as this function is invoked. + Arguments: + args: list of arguments Returns: argparse namespace """ @@ -235,7 +239,7 @@ def loadOptions(*args): # command line options = parser.parse_args() else: - options = parser.parse_args(list(args)) + options = parser.parse_args(args) # adding specified filenames to arguments options.filenames.append(options.input_pdb) From f13d3d812125da554f5d793ee8716d581413450f Mon Sep 17 00:00:00 2001 From: Nathan Baker Date: Thu, 21 May 2020 15:50:23 -0700 Subject: [PATCH 5/5] PROPKA testing working with pytest. --- .gitignore | 4 +- README.md | 11 ++- requirements.txt | 1 + tests/README.md | 15 ++++ tests/__init__.py | 0 tests/hybrid36.py | 57 ---------------- tests/pkacalc_test.py | 98 -------------------------- tests/regression_test.py | 144 +++++++++++++++++++++++++++++++++++++++ tests/runtest.py | 68 ------------------ 9 files changed, 171 insertions(+), 227 deletions(-) create mode 100644 requirements.txt create mode 100644 tests/README.md delete mode 100644 tests/__init__.py delete mode 100644 tests/hybrid36.py delete mode 100644 tests/pkacalc_test.py create mode 100644 tests/regression_test.py delete mode 100644 tests/runtest.py diff --git a/.gitignore b/.gitignore index 97a38a4..398bcbe 100644 --- a/.gitignore +++ b/.gitignore @@ -1,5 +1,5 @@ # Compiled python files *.py[cod] - - +# PIP stuff +*.egg-info diff --git a/README.md b/README.md index 699db18..3b6fb0f 100644 --- a/README.md +++ b/README.md @@ -36,10 +36,16 @@ directory: python setup.py install --user --install-scripts ~/bin +For the purposes of testing or development, you may prefer to install PROPKA as +an editable module via PIP by running +``` +pip install -e . +``` +from within a virtual environment (e.g., via [virtualenv](https://pypi.org/project/virtualenv/)). ## Requirements -* Python 2.7 or higher or Python 3.1 or higher +* Python 3.1 or higher ## Getting started @@ -56,7 +62,8 @@ Calculate using pdb file ## Testing (for developers) -Please run `Tests/pkacalc_test.py` via `unittest` or `pytest` as well as `python Tests/runtest.py` after changes before pushing commits. +Please see [`tests/README.md`](tests/README.md) for testing instructions. +Please run these tests after making changes to the code and _before_ pushing commits. ## References / Citations diff --git a/requirements.txt b/requirements.txt new file mode 100644 index 0000000..55b033e --- /dev/null +++ b/requirements.txt @@ -0,0 +1 @@ +pytest \ No newline at end of file diff --git a/tests/README.md b/tests/README.md new file mode 100644 index 0000000..c1b9717 --- /dev/null +++ b/tests/README.md @@ -0,0 +1,15 @@ +# Testing PROPKA + +These tests assume that PROPKA is installed as a module on your system. +If you are running in a virtual environment and want to make changes to your +code, module installation accomplished by +``` +pip install -e . +``` +from the top level of the PROPKA source directory. + +Once PROPKA is available as a module, the tests can be run by +``` +python -m pytest tests +``` +either in the top-level directory or `tests` subdirectory. diff --git a/tests/__init__.py b/tests/__init__.py deleted file mode 100644 index e69de29..0000000 diff --git a/tests/hybrid36.py b/tests/hybrid36.py deleted file mode 100644 index 4429e07..0000000 --- a/tests/hybrid36.py +++ /dev/null @@ -1,57 +0,0 @@ -import unittest - -import propka.hybrid36 as hybrid36 - -class Hybrid36Test(unittest.TestCase): - def testDecode(self): - test_values = { - "99999": 99999, - "A0000": 100000, - "0": 0, - "9": 9, - "A": 10, - " ZZZZY": 43770014, - "ZZZZZ": 43770015, # ZZZZZ - A0000 + 100000 - "a0000": 43770016, - "zzzzz": 87440031, - "zzzzy": 87440030, - "99": 99, - "A0": 100, - "ZZ": 1035, - "zz": 1971, - "-99999": -99999, - "-A0000": -100000, - "-0": 0, - "-9": -9, - "-A": -10, - "-ZZZZY": -43770014, - "-ZZZZZ": -43770015, # ZZZZZ - A0000 + 100000 - "-a0000": -43770016, - "-zzzzz": -87440031, - "-zzzzy": -87440030, - "-99": -99, - "-A0": -100, - "-ZZ": -1035, - "-zz": -1971, - "PROPKA": 954495146, - "A001Z": 100071, - "B0000": 1779616, - } - - for k, v in test_values.iteritems(): - self.assertEqual(hybrid36.decode(k), v) - - def testErrors(self): - test_values = [ - "99X99", - "X9-99", - "XYZa", - "", - "-", - "!NotOk", - ] - - for v in test_values: - with self.assertRaises(ValueError) as e: - hybrid36.decode(v) - self.assertTrue(v in str(e.exception)) diff --git a/tests/pkacalc_test.py b/tests/pkacalc_test.py deleted file mode 100644 index 5a2f5ca..0000000 --- a/tests/pkacalc_test.py +++ /dev/null @@ -1,98 +0,0 @@ -"""Reproduce tests from original PROPKA.""" -import os -import re -from subprocess import call -import sys -import unittest -import logging -import propka.lib -import propka.molecular_container - - -# This error tolerance was chosen to make Ubuntu 18.04 pass under Windows -# Subsystem Linux. -ACCEPTABLE_ERROR = 0.001 - - -def run_propka(args): - """Run PROPKA. - - Args: - args: command-line arguments for PROPKA - """ - options = propka.lib.loadOptions() - raise NotImplementedError("Need to incorporated command-line arguments") - pdbfiles = options.filenames - - for pdbfile in pdbfiles: - my_molecule = propka.molecular_container.Molecular_container(pdbfile, options) - my_molecule.calculate_pka() - my_molecule.write_pka() - - -# Setting this up as a direct translation of the original runtest.py script -# that will be run as part of 'python setup.py test'. This takes on the -# order of 10s to run. - -class SystemTest(unittest.TestCase): - """ - Run the program and compare against reference results. - """ - def test_pka_calc(self): - pdbs = ['1FTJ-Chain-A', - '1HPX', - '4DFR'] - - test_dir = os.path.dirname(__file__) - base_dir = os.path.dirname(test_dir) - - executable = os.path.join(base_dir, "scripts", "propka31.py") - - env = { "PYTHONPATH" : base_dir } - - for pdb in pdbs: - input_filename = os.path.join(test_dir, "pdb", pdb + ".pdb") - output_filename = os.path.join(test_dir, pdb + ".out") - - output_file = open(output_filename, "w") - call([sys.executable, executable, input_filename], - stdout=output_file, env=env) - output_file.close() - - # Check pka predictions. - ref = open(os.path.join(test_dir, "results", pdb + ".dat")) - output = open(output_filename) - - atpka = False - errors = [] - for line in output: - if not atpka: - # Start testing pka values. - if "model-pKa" in line: - atpka = True - continue - - m = re.search('([0-9]+\.[0-9]+)', line) - if not m: - break - - expected_value = float(ref.readline()) - value = float(m.group(0)) - value_error = (value-expected_value)/expected_value - - if abs(value_error) > ACCEPTABLE_ERROR: - logging.error(value_error) - identity = line[:m.start()].strip() - errors.append("%12s %8.2f %8.2f" % - (identity, expected_value, value)) - - os.remove("%s.pka" % pdb) - os.remove("%s.propka_input" % pdb) - - ref.close() - output.close() - - if errors: - error_header = " Group Expected Calculated\n" - self.fail("Unexpected pKa values:\n" + error_header + - "\n".join(errors)) diff --git a/tests/regression_test.py b/tests/regression_test.py new file mode 100644 index 0000000..f90f009 --- /dev/null +++ b/tests/regression_test.py @@ -0,0 +1,144 @@ +"""Tests for PROPKA 3.1""" +import logging +import os +import re +from pathlib import Path +import pytest +import pandas as pd +import propka.lib +import propka.molecular_container + + +_LOGGER = logging.getLogger(__name__) + + +# Maximum error set by number of decimal places in pKa output as well as need +# to make unmodified code work on WSL Ubuntu 18.04 +MAX_ERR = 0.01 + + +# This directory +TEST_DIR = Path("tests") +# Location for test PDBs +PDB_DIR = Path("pdb") +# Location for results for comparing output (allow running from tests/ and ../tests/) +RESULTS_DIR = Path("tests/results") +if not RESULTS_DIR.is_dir(): + _LOGGER.warning("Switching to sub-directory") + RESULTS_DIR = Path("results") +# Arguments to add to all tests +DEFAULT_ARGS = [] + + +def get_test_dirs(): + """Get locations of test files. + + Returns: + dictionary with test file locations. + """ + path_dict = {} + for key, path in [("pdbs", PDB_DIR), ("results", RESULTS_DIR)]: + test_path = TEST_DIR / path + if test_path.is_dir(): + path_dict[key] = test_path + else: + test_path = path + if test_path.is_dir(): + path_dict[key] = test_path + else: + errstr = "Can't find %s test files in %s" % (key, [TEST_DIR / path, path]) + raise FileNotFoundError(errstr) + return path_dict + + +def run_propka(options, pdb_path, tmp_path): + """Run PROPKA software. + + Args: + options: list of PROPKA options + pdb_path: path to PDB file + tmp_path: path for working directory + """ + options += [str(pdb_path)] + args = propka.lib.loadOptions(options) + try: + _LOGGER.warning("Working in tmpdir %s because of PROPKA file output; need to fix this.", + tmp_path) + cwd = Path.cwd() + os.chdir(tmp_path) + molecule = propka.molecular_container.Molecular_container(str(pdb_path), args) + molecule.calculate_pka() + molecule.write_pka() + except Exception as err: + raise err + finally: + os.chdir(cwd) + + +def compare_output(pdb, tmp_path, ref_path): + """Compare results of test with reference. + + Args: + pdb: PDB filename stem + tmp_path: temporary directory + ref_path: path with reference results + Raises: + ValueError if results disagree. + """ + ref_data = [] + with open(ref_path, "rt") as ref_file: + for line in ref_file: + ref_data.append(float(line)) + + test_data = [] + pka_path = Path(tmp_path) / ("%s.pka" % pdb) + with open(pka_path, "rt") as pka_file: + at_pka = False + for line in pka_file: + if not at_pka: + if "model-pKa" in line: + at_pka = True + elif line.startswith("---"): + at_pka = False + else: + m = re.search(r'([0-9]+\.[0-9]+)', line) + value = float(m.group(0)) + test_data.append(value) + + df = pd.DataFrame({"reference": ref_data, "test": test_data}) + df["difference"] = df["reference"] - df["test"] + max_err = df["difference"].abs().max() + if max_err > MAX_ERR: + errstr = "Error in test (%g) exceeds maximum (%g)" % (max_err, MAX_ERR) + raise ValueError(errstr) + + +@pytest.mark.parametrize("pdb, options", [ + pytest.param("1FTJ-Chain-A", [], id="1FTJ-Chain-A: no options"), + pytest.param('1HPX', [], id="1HPX: no options"), + pytest.param('4DFR', [], id="4DFR: no options"), + pytest.param('3SGB', [], id="3SGB: no options"), + pytest.param('3SGB-subset', ["--titrate_only", + "E:17,E:18,E:19,E:29,E:44,E:45,E:46,E:118,E:119,E:120,E:139"], + id="3SGB: --titrate_only"), + pytest.param('1HPX-warn', ['--quiet'], id="1HPX-warn: --quiet")]) +def test_regression(pdb, options, tmp_path): + """Basic regression test of PROPKA functionality.""" + path_dict = get_test_dirs() + ref_path = path_dict["results"] / ("%s.dat" % pdb) + if ref_path.is_file(): + ref_path = ref_path.resolve() + else: + _LOGGER.warning("Missing results file for comparison: %s", ref_path) + ref_path = None + pdb_path = path_dict["pdbs"] / ("%s.pdb" % pdb) + if pdb_path.is_file(): + pdb_path = pdb_path.resolve() + else: + errstr = "Missing PDB file: %s" % pdb_path + raise FileNotFoundError(errstr) + tmp_path = Path(tmp_path).resolve() + + run_propka(options, pdb_path, tmp_path) + if ref_path is not None: + compare_output(pdb, tmp_path, ref_path) diff --git a/tests/runtest.py b/tests/runtest.py deleted file mode 100644 index 0a3f351..0000000 --- a/tests/runtest.py +++ /dev/null @@ -1,68 +0,0 @@ -#!/usr/bin/env python - -""" Run test for test pdbs """ - -from __future__ import division -from __future__ import print_function - -from subprocess import call -import os, re -import sys - -if __name__ == "__main__": - # A list of input structures and command-line arguments to be passed in - # to PROPKA for each: - pdbs = [('1FTJ-Chain-A', []), - ('1HPX', []), - ('4DFR', []), - ('3SGB', []), - ('3SGB-subset', ['--titrate_only', 'E:17,E:18,E:19,E:29,E:44,E:45,E:46,E:118,E:119,E:120,E:139']), - ('1HPX-warn', ['--quiet']), - ] - - for pdb, args in pdbs: - print('') - print('RUNNING '+pdb) - - # Run pka calculation - fh = open(pdb + '.out', 'w') - cmd = [sys.executable, '../scripts/propka31.py','pdb/%s.pdb' % pdb] + args - ret = call(cmd, stdout=fh, stderr=fh) - if ret != 0: - print(" ERR:") - print(" Failed to execute PROPKA on %s" % pdb) - print(" See: %s.out" % pdb) - sys.exit(1) - - # Test pka predictions - result_file = 'results/%s.dat' % pdb - if not os.path.isfile(result_file): - print(" ERR:") - print(" file not found: %s" % result_file) - sys.exit(1) - - result = open(result_file,'r') - atpka = False - for line in open(pdb+'.pka', 'r').readlines(): - if not atpka: - if "model-pKa" in line: - # test pka - atpka = True - continue - else: - continue - if "-" in line: - # done testing - atpka = False - continue - - expected_value = float(result.readline()) - m = re.search('([0-9]+\.[0-9]+)', line) - value = float(m.group(0)) - - if value != expected_value: - print(" ERR:") - print(line) - print(" %s should be: %s" % (value, expected_value)) - sys.exit(1) -