Repository: marrink-lab/vermouth-martinize

Issue:
run_dssp doesn't work on Windows
The DSSP processor doesn't work on windows, since it tries the following:
```python3
process = subprocess.Popen(
        [executable, "-i", "/dev/stdin"],
        stderr=subprocess.PIPE,
        stdout=subprocess.PIPE,
        stdin=subprocess.PIPE
    )
```
In other words, it tries telling dssp to read the protein from stdin. Windows being what it is, this doesn't work so well and blows up with a "No such file" error. 

My suggestion would be to write the atomistic structure to a temporary file, and feed that to dssp.

Discussion:

Patch:
diff --git a/requirements-docs.txt b/requirements-docs.txt
index 27f3ca8..19fd51c 100644
--- a/requirements-docs.txt
+++ b/requirements-docs.txt
@@ -1,4 +1,4 @@
-sphinx >= 1.8.0
+sphinx == 3.1.2
 sphinxcontrib-apidoc
 pbr
 setuptools >= 30.3.0
diff --git a/vermouth/dssp/dssp.py b/vermouth/dssp/dssp.py
index 8b834d1..169dccd 100644
--- a/vermouth/dssp/dssp.py
+++ b/vermouth/dssp/dssp.py
@@ -17,15 +17,19 @@ Assign protein secondary structures using DSSP.
 """
 
 import collections
+import logging
 import os
 import subprocess
-import logging
+import tempfile
 
 from ..pdb import pdb
 from ..system import System
 from ..processors.processor import Processor
 from ..selectors import is_protein, selector_has_position, filter_minimal, select_all
 from .. import utils
+from ..log_helpers import StyleAdapter, get_logger
+
+LOGGER = StyleAdapter(get_logger(__name__))
 
 
 class DSSPError(Exception):
@@ -184,23 +188,36 @@ def run_dssp(system, executable='dssp', savefile=None):
     read_dssp2
         Parse a DSSP output.
     """
-    process = subprocess.Popen(
-        [executable, "-i", "/dev/stdin"],
-        stderr=subprocess.PIPE,
+    tmpfile_handle, tmpfile_name = tempfile.mkstemp(suffix='.pdb', text=True,
+                                                    dir='.', prefix='dssp_in_')
+    tmpfile_handle = os.fdopen(tmpfile_handle, mode='w')
+    tmpfile_handle.write(pdb.write_pdb_string(system, conect=False))
+    tmpfile_handle.close()
+
+    process = subprocess.run(
+        [executable, '-i', tmpfile_name],
         stdout=subprocess.PIPE,
-        stdin=subprocess.PIPE
-    )
-    out, err = process.communicate(
-        pdb.write_pdb_string(system, conect=False).encode('utf8')
+        stderr=subprocess.PIPE,
+        check=False,
+        universal_newlines=True
     )
-    out = out.decode('utf8')
-    status = process.wait()
+
+    status = process.returncode
+    # If an error is encountered, or the loglevel is low enough, preserve the
+    # DSSP input file, and print a nice message.
+    if not status and LOGGER.getEffectiveLevel() > logging.DEBUG:
+        os.remove(tmpfile_name)
     if status:
-        raise DSSPError(err.decode('utf8'))
+        message = 'DSSP encountered an error. The message was {err}. The input' \
+                  ' file provided to DSSP can be found at {file}.'
+        raise DSSPError(message.format(err=process.stderr, file=tmpfile_name))
+    else:
+        LOGGER.debug('DSSP input file written to {}', tmpfile_name)
+
     if savefile is not None:
         with open(str(savefile), 'w') as outfile:
-            outfile.write(out)
-    return read_dssp2(out.split('\n'))
+            outfile.write(process.stdout)
+    return read_dssp2(process.stdout.split('\n'))
 
 
 def _savefile_path(molecule, savedir=None):
@@ -420,7 +437,7 @@ def convert_dssp_annotation_to_martini(
         # as it *could* be due to the DSSP assignation having been skipped
         # for some reason.
         msg = 'No DSSP assignation to convert to Martini secondary structure intermediates.'
-        logging.debug(msg)
+        LOGGER.debug(msg)
     else:
         # This is more of a problem. For now, we do not know what to do with
         # incomplete DSSP assignation. This may come later as a problem if


Test patch:
diff --git a/vermouth/tests/helper_functions.py b/vermouth/tests/helper_functions.py
index f2d0c36..d199bc3 100644
--- a/vermouth/tests/helper_functions.py
+++ b/vermouth/tests/helper_functions.py
@@ -16,6 +16,7 @@
 Contains helper functions for tests.
 """
 import operator
+import os
 import networkx.algorithms.isomorphism as iso
 
 
@@ -57,3 +58,24 @@ def equal_graphs(g1, g2,
         edge_equal = iso.categorical_node_match(edge_attrs, [''] * len(edge_attrs))
     matcher = iso.GraphMatcher(g1, g2, node_match=node_equal, edge_match=edge_equal)
     return matcher.is_isomorphic()
+
+
+def find_in_path(names=('martinize2', 'martinize2.py')):
+    """
+    Finds and returns the location of one of `names` in PATH, and returns the
+    first match.
+
+    Parameters
+    ----------
+    names: collections.abc.Sequence
+        Names to look for in PATH.
+
+    Returns
+    -------
+    os.PathLike or None
+    """
+    for folder in os.getenv("PATH", '').split(os.pathsep):
+        for name in names:
+            fullpath = os.path.join(folder, name)
+            if os.path.isfile(fullpath):
+                return fullpath
\ No newline at end of file
diff --git a/vermouth/tests/test_dssp.py b/vermouth/tests/test_dssp.py
index 3309354..650dcbb 100644
--- a/vermouth/tests/test_dssp.py
+++ b/vermouth/tests/test_dssp.py
@@ -17,6 +17,7 @@ Test the functions required to use DSSP.
 """
 
 import os
+import glob
 import itertools
 
 import numpy as np
@@ -25,10 +26,12 @@ import pytest
 import vermouth
 from vermouth.forcefield import get_native_force_field
 from vermouth.dssp import dssp
+from vermouth.dssp.dssp import DSSPError
 from vermouth.pdb.pdb import read_pdb
 from vermouth.tests.datafiles import (
     PDB_PROTEIN,
     DSSP_OUTPUT,
+    PDB_ALA5_CG,
 )
 
 DSSP_EXECUTABLE = os.environ.get('VERMOUTH_TEST_DSSP', 'dssp')
@@ -319,7 +322,7 @@ def test_run_dssp(savefile, tmpdir):
     generate a save file only if requested.
     """
     # The test runs twice, once with the savefile set to True so we test with
-    # savinf the DSSP output to file, and once with savefile set t False so we
+    # saving the DSSP output to file, and once with savefile set t False so we
     # do not generate the file. The "savefile" argument is set by
     # pytest.mark.parametrize.
     # The "tmpdir" argument is set by pytest and is the path to a temporary
@@ -361,6 +364,37 @@ def test_run_dssp(savefile, tmpdir):
         assert not os.listdir(str(tmpdir))
 
 
+@pytest.mark.parametrize('pdb, loglevel,expected', [
+    (PDB_PROTEIN, 10, True),  # DEBUG
+    (PDB_PROTEIN, 30, False),  # WARNING
+    # Using a CG pdb will cause a DSSP error, which should preserve the input
+    (PDB_ALA5_CG, 10, True),  # DEBUG
+    (PDB_ALA5_CG, 30, True),  # WARNING
+])
+def test_run_dssp_input_file(tmpdir, caplog, pdb, loglevel, expected):
+    """
+    Test that the DSSP input file is preserved (only) in the right conditions
+    """
+    caplog.set_level(loglevel)
+    system = vermouth.System()
+    for molecule in read_pdb(str(pdb)):
+        system.add_molecule(molecule)
+    with tmpdir.as_cwd():
+        try:
+            dssp.run_dssp(system, executable=DSSP_EXECUTABLE)
+        except DSSPError:
+            pass
+        if expected:
+            target = 1
+        else:
+            target = 0
+        matches = glob.glob('dssp_in*.pdb')
+        assert len(matches) == target, matches
+        if matches:
+            # Make sure it's a valid PDB file. Mostly anyway.
+            list(read_pdb(matches[0]))
+
+
 def test_cterm_atomnames():
     nodes = [
         dict(resname="ALA", atomname="N", element='N', resid=1, chain="", position=np.array([9.534, 5.359, 0.000])),
diff --git a/vermouth/tests/test_name_moltype.py b/vermouth/tests/test_name_moltype.py
index 8687324..f962328 100644
--- a/vermouth/tests/test_name_moltype.py
+++ b/vermouth/tests/test_name_moltype.py
@@ -27,6 +27,7 @@ import itertools
 from glob import glob
 import os.path
 import subprocess
+import sys
 import pytest
 from hypothesis import given, assume
 from hypothesis import strategies as st
@@ -34,6 +35,7 @@ from vermouth import System
 from vermouth.processors.name_moltype import NameMolType
 from .molecule_strategies import random_molecule
 from .datafiles import PDB_HB
+from .helper_functions import find_in_path
 
 @st.composite
 def molecules_and_moltypes(draw, max_moltypes=4, min_size=0, max_size=None,
@@ -140,8 +142,11 @@ def test_martinize2_moltypes(tmpdir, deduplicate):
     """
     Run martinize2 and make sure the ITP file produced have the expected names.
     """
+    martinize2 = find_in_path()
+
     command = [
-        'martinize2',
+        sys.executable,
+        martinize2,
         '-f', str(PDB_HB),
         '-o', 'topol.top',
         '-x', 'out.pdb',
@@ -155,9 +160,8 @@ def test_martinize2_moltypes(tmpdir, deduplicate):
         n_outputs = 4
     expected = ['molecule_{}.itp'.format(i) for i in range(n_outputs)]
 
-    proc = subprocess.Popen(command, cwd=str(tmpdir))
-    exit_code = proc.wait(timeout=90)
-    assert exit_code == 0
+    proc = subprocess.run(command, cwd=str(tmpdir), timeout=90, check=False)
+    assert proc.returncode == 0
 
     itp_files = sorted(os.path.basename(fname) for fname in glob(str(tmpdir / '*.itp')))
     assert itp_files == expected