Merge pull request #6 from dgasmith/cleanups

General Cleanup

README.md

@@ -5,6 +5,7 @@ QCEngine
 [![Language grade: Python](https://img.shields.io/lgtm/grade/python/g/MolSSI/QCEngine.svg?logo=lgtm&logoWidth=18)](https://lgtm.com/projects/g/MolSSI/QCEngine/context:python)
 [![Documentation Status](https://readthedocs.org/projects/qcengine/badge/?version=latest)](https://qcengine.readthedocs.io/en/latest/?badge=latest)
 [![Anaconda-Server Badge](https://anaconda.org/molssi/qcengine/badges/version.svg)](https://anaconda.org/molssi/qcengine)
+[![Chat on Slack](https://img.shields.io/badge/chat-on_slack-green.svg?longCache=true&style=flat&logo=slack)](https://join.slack.com/t/qcdb/shared_invite/enQtNDIzNTQ2OTExODk0LWM3OTgxN2ExYTlkMTlkZjA0OTExZDlmNGRlY2M4NWJlNDlkZGQyYWUxOTJmMzc3M2VlYzZjMjgxMDRkYzFmOTE)
 
 
 Quantum chemistry program executor and IO standardizer ([QCSchema](https://github.com/MolSSI/QC_JSON_Schema)) for quantum chemistry. See the [documentation](https://qcengine.readthedocs.io/en/latest/) for more information.

qcengine/compute.py

@@ -5,15 +5,14 @@
 import copy
 import time
 
-from . import config
 from . import util
 
 # Single computes
 from . import psi_compute
 from . import rdkit_compute
 
 
-def compute(input_data, program, raise_error=False):
+def compute(input_data, program, raise_error=False, capture_output=True):
     """Executes a single quantum chemistry program given a QC Schema input.
 
     The full specification can be found at:
@@ -25,8 +24,10 @@ def compute(input_data, program, raise_error=False):
         A QC Schema input specification
     program : {"psi4", "rdkit"}
         The program to run the input under
-    raise_error : bool, option
+    raise_error : bool, optional
         Determines if compute should raise an error or not.
+    capture_output : bool, optional
+        Determines if stdout/stderr should be captured.
 
     Returns
     -------
@@ -37,30 +38,20 @@ def compute(input_data, program, raise_error=False):
     input_data = copy.deepcopy(input_data)
 
     # Run the program
-    comp_time = time.time()
-    if program == "psi4":
-        output_data = psi_compute.run_psi4(input_data)
-    elif program == "rdkit":
-        output_data = rdkit_compute.run_rdkit(input_data)
-    else:
-        output_data["error"] = "QCEngine: Program {} not understood".format(program)
-    comp_time = time.time() - comp_time
-
-    # Raise an error if one exists and a user requested a raise
-    if raise_error and ("error" in output_data) and (output_data["error"] is not False):
-        raise ValueError(output_data["error"])
-
-    # Fill out provenance datadata
-    if "provenance" in output_data:
-        output_data["provenance"].update(config.get_provenance())
-    else:
-        output_data["provenance"] = config.get_provenance()
-
-    output_data["provenance"]["wall_time"] = comp_time
-
-    return output_data
-
-def compute_procedure(input_data, procedure, raise_error=False):
+    with util.compute_wrapper(capture_output=capture_output) as metadata:
+        if program == "psi4":
+            output_data = psi_compute.run_psi4(input_data)
+        elif program == "rdkit":
+            output_data = rdkit_compute.run_rdkit(input_data)
+        else:
+            output_data = input_data
+            output_data["success"] = False
+            output_data["error_message"] = "QCEngine Call Error:\nProgram {} not understood".format(program)
+
+    return util.handle_output_metadata(output_data, metadata, raise_error=raise_error)
+
+
+def compute_procedure(input_data, procedure, raise_error=False, capture_output=True):
     """Runs a procedure (a collection of the quantum chemistry executions)
 
     Parameters
@@ -71,6 +62,8 @@ def compute_procedure(input_data, procedure, raise_error=False):
         The name of the procedure to run
     raise_error : bool, option
         Determines if compute should raise an error or not.
+    capture_output : bool, optional
+        Determines if stdout/stderr should be captured.
 
     Returns
     ------
@@ -81,24 +74,12 @@ def compute_procedure(input_data, procedure, raise_error=False):
     input_data = copy.deepcopy(input_data)
 
     # Run the procedure
-    comp_time = time.time()
-    if procedure == "geometric":
-        output_data = util.get_module_function("geometric", "run_json.geometric_run_json")(input_data)
-    else:
-        output_data["error"] = "QCEngine: Procedure {} not understood".format(procedure)
-    comp_time = time.time() - comp_time
-
-    # Raise an error if one exists and a user requested a raise
-    if raise_error and ("error" in output_data) and (output_data["error"] is not False):
-        raise ValueError(output_data["error"])
-
-    # Fill out provenance datadata
-    if "provenance" in output_data:
-        output_data["provenance"].update(config.get_provenance())
-    else:
-        output_data["provenance"] = config.get_provenance()
-
-    output_data["provenance"]["wall_time"] = comp_time
-
-
-    return output_data
+    with util.compute_wrapper(capture_output=capture_output) as metadata:
+        if procedure == "geometric":
+            output_data = util.get_module_function("geometric", "run_json.geometric_run_json")(input_data)
+        else:
+            output_data = input_data
+            output_data["success"] = False
+            output_data["error_message"] = "QCEngine Call Error:\nProcedure {} not understood".format(program)
+
+    return util.handle_output_metadata(output_data, metadata, raise_error=raise_error)

qcengine/psi_compute.py

@@ -119,7 +119,7 @@ def run_psi4(input_data):
         if output_data is False:
             output_data["success"] = False
             if "error" not in rjson:
-                output_data["error"] = "Unspecified error occured."
+                output_data["error_message"] = "Unspecified error occured."
 
         output_data["molecule"] = json_mol
 

qcengine/rdkit_compute.py

@@ -22,11 +22,11 @@ def run_rdkit(ret_data):
 
     # Handle errors
     if ("molecular_charge" in jmol) and (abs(jmol["molecular_charge"]) < 1.e-6):
-        ret_data["error"] = "run_rdkit does not currently support charged molecules"
+        ret_data["error_message"] = "run_rdkit does not currently support charged molecules"
         return ret_data
 
     if "connectivity" not in jmol:
-        ret_data["error"] = "run_rdkit molecule must have a connectivity graph"
+        ret_data["error_message"] = "run_rdkit molecule must have a connectivity graph"
         return ret_data
 
     # Build out the base molecule
@@ -57,11 +57,11 @@ def run_rdkit(ret_data):
         ff = AllChem.UFFGetMoleculeForceField(mol)
         all_params = AllChem.UFFHasAllMoleculeParams(mol)
     else:
-        ret_data["error"] = "run_rdkit can only accepts UFF methods"
+        ret_data["error_message"] = "run_rdkit can only accepts UFF methods"
         return ret_data
 
     if all_params is False:
-        ret_data["error"] = "run_rdkit did not match all parameters to molecule"
+        ret_data["error_message"] = "run_rdkit did not match all parameters to molecule"
         return ret_data
 
     ff.Initialize()
@@ -74,7 +74,7 @@ def run_rdkit(ret_data):
         coef = 1 / (units.bohr_to_angstrom * units.hartree_to_kj_mol)
         ret_data["return_result"] = [x * coef for x in ff.CalcGrad()]
     else:
-        ret_data["error"] = "run_rdkit did not understand driver method '{}'.".format(ret_data["driver"])
+        ret_data["error_message"] = "run_rdkit did not understand driver method '{}'.".format(ret_data["driver"])
         return ret_data
 
     ret_data["provenance"] = {

qcengine/tests/test_compute.py

@@ -11,6 +11,12 @@
 _base_json = {"schema_name": "qc_schema_input", "schema_version": 1}
 
 
+def test_missing_key():
+    ret = dc.compute({"hello": "hi"}, "bleh")
+    assert ret["success"] is False
+    assert "hello" in ret
+
+
 @addons.using_psi4
 def test_psi4_task():
     json_data = copy.deepcopy(_base_json)
@@ -71,7 +77,7 @@ def test_rdkit_connectivity_error():
 
     ret = dc.compute(json_data, "rdkit")
     assert ret["success"] is False
-    assert "conn" in ret["error"]
+    assert "connectivity" in ret["error_message"]
 
     with pytest.raises(ValueError):
         ret = dc.compute(json_data, "rdkit", raise_error=True)

qcengine/tests/test_procedures.py

@@ -56,6 +56,22 @@ def test_geometric_psi4():
     geom = ret["final_molecule"]["geometry"]
     assert pytest.approx(_bond_dist(geom, 0, 1), 1.e-4) == 1.3459150737
 
+@addons.using_rdkit
+@addons.using_geometric
+def test_geometric_stdout():
+    inp = copy.deepcopy(_base_json)
+
+    inp["initial_molecule"] = dc.get_molecule("water")
+    inp["input_specification"]["model"] = {"method": "UFF", "basis": ""}
+    inp["keywords"]["program"] = "rdkit"
+
+    ret = dc.compute_procedure(inp, "geometric")
+    assert ret["success"] is True
+    assert "Converged!" in ret["stdout"]
+    assert ret["stderr"] == "No stderr recieved."
+
+    with pytest.raises(ValueError):
+        ret = dc.compute_procedure(inp, "rdkit", raise_error=True)
 
 @addons.using_rdkit
 @addons.using_geometric
@@ -69,7 +85,7 @@ def test_geometric_rdkit_error():
 
     ret = dc.compute_procedure(inp, "geometric")
     assert ret["success"] is False
-    assert "conn" in ret["error"]
+    assert isinstance(ret["error_message"], str)
 
     with pytest.raises(ValueError):
         ret = dc.compute_procedure(inp, "rdkit", raise_error=True)

qcengine/util.py

@@ -2,11 +2,56 @@
 Several import utilities
 """
 
+from contextlib import contextmanager
+
+import traceback
+import time
 import importlib
+import io
+import sys
 import operator
 
+from . import config
+
+__all__ = ["compute_wrapper", "get_module_function"]
+
+@contextmanager
+def compute_wrapper(capture_output=True):
+    """Wraps compute for timing, output capturing, and raise protection
+    """
+
+    ret = {"stdout": "", "stderr": ""}
+
+    # Start timer
+    comp_time = time.time()
+
+    # Capture stdout/err
+    if capture_output:
+        new_stdout = io.StringIO("No stdout recieved.")
+        new_stderr = io.StringIO("No stderr recieved.")
+
+        old_stdout, sys.stdout = sys.stdout, new_stdout
+        old_stderr, sys.stderr = sys.stderr, new_stderr
+
+    try:
+        yield ret
+        ret["success"] = True
+    except Exception as e:
+        ret["error_message"] = "QCEngine Call Error:\n" + traceback.format_exc()
+        ret["success"] = False
+
+    # Place data
+    ret["wall_time"] = time.time() - comp_time
+    ret["stdout"] = new_stdout.getvalue()
+    ret["stderr"] = new_stderr.getvalue()
+
+    # Replace stdout/err
+    if capture_output:
+        sys.stdout = old_stdout
+        sys.stderr = old_stderr
+
 def get_module_function(module, func_name, subpackage=None):
-    """Summary
+    """Obtains a function from a given string
 
     Parameters
     ----------
@@ -34,3 +79,25 @@ def get_module_function(module, func_name, subpackage=None):
     pkg = importlib.import_module(module, subpackage)
 
     return operator.attrgetter(func_name)(pkg)
+
+def handle_output_metadata(output_data, metadata, raise_error=False):
+
+    output_data["stdout"] = metadata["stdout"]
+    output_data["stderr"] = metadata["stderr"]
+    if metadata["success"] is not True:
+        output_data["success"] = False
+        output_data["error_message"] = metadata["error_message"]
+
+    # Raise an error if one exists and a user requested a raise
+    if raise_error and (output_data["success"] is not True):
+        raise ValueError(output_data["error_message"])
+
+    # Fill out provenance datadata
+    if "provenance" in output_data:
+        output_data["provenance"].update(config.get_provenance())
+    else:
+        output_data["provenance"] = config.get_provenance()
+
+    output_data["provenance"]["wall_time"] = metadata["wall_time"]
+
+    return output_data

setup.cfg

@@ -7,6 +7,10 @@ omit =
     */tests/*
     qcengine/_version.py
 
+[tool:pytest]
+filterwarnings =
+    ignore::DeprecationWarning
+    ignore::PendingDeprecationWarning
 
 [yapf]
 # YAPF, in .style.yapf files this shows up as "[style]" header