Merge pull request #159 from marrink-lab/ptm_node_match

Remove needless GraphMatcher subclasses

doc/source/conf.py

@@ -82,9 +82,7 @@
 
 
 nitpick_ignore = [
-        ('py:class', 'networkx.algorithms.isomorphism.vf2userfunc.GraphMatcher'),
         ('py:class', 'networkx.algorithms.isomorphism.isomorphvf2.GraphMatcher'),
-        ('py:class', 'networkx.classes.graph.Graph'),
         ]
 
 # -- Options for HTML output -------------------------------------------------
@@ -187,8 +185,41 @@
 # -- Options for intersphinx extension ---------------------------------------
 # Example configuration for intersphinx: refer to the Python standard library.
 intersphinx_mapping = {
-                       'python': ('https://docs.python.org/', None),
+                       'python': ('https://docs.python.org', None),
                        'networkx': ('https://networkx.github.io/documentation/latest', None),
-                       'numpy': ('http://docs.scipy.org/doc/numpy/', None),
+                       'numpy': ('http://docs.scipy.org/doc/numpy', None),
                        'scipy': ('http://docs.scipy.org/doc/scipy/reference', None),
                       }
+
+
+# Borrowed from https://github.com/sphinx-doc/sphinx/issues/5603
+# On top of that, networkx.isomorphism.GraphMatcher is not documented, so link
+# to the VF2 isomorphism module instead.
+# See https://github.com/networkx/networkx/issues/3239
+intersphinx_aliases = {
+        ('py:class', 'networkx.classes.graph.Graph'): ('py:class', 'networkx.Graph'),
+        #('py:class', 'networkx.algorithms.isomorphism.vf2userfunc.GraphMatcher'): ('py:class', 'networkx.isomorphism.GraphMatcher'),
+        ('py:class', 'networkx.algorithms.isomorphism.vf2userfunc.GraphMatcher'): ('py:module', 'networkx.algorithms.isomorphism.isomorphvf2'),
+        ('py:class', 'networkx.isomorphism.GraphMatcher'): ('py:module', 'networkx.algorithms.isomorphism.isomorphvf2')
+        }
+
+def add_intersphinx_aliases_to_inv(app):
+    from sphinx.ext.intersphinx import InventoryAdapter
+    inventories = InventoryAdapter(app.builder.env)
+
+    for alias, target in app.config.intersphinx_aliases.items():
+        alias_domain, alias_name = alias
+        target_domain, target_name = target
+        try:
+            found = inventories.main_inventory[target_domain][target_name]
+            try:
+                inventories.main_inventory[alias_domain][alias_name] = found
+            except KeyError:
+                continue
+        except KeyError:
+            continue
+
+
+def setup(app):
+    app.add_config_value('intersphinx_aliases', {}, 'env')
+    app.connect('builder-inited', add_intersphinx_aliases_to_inv)

vermouth/processors/canonicalize_modifications.py

@@ -32,32 +32,22 @@
 LOGGER = StyleAdapter(get_logger(__name__))
 
 
-class PTMGraphMatcher(nx.isomorphism.GraphMatcher):
+def ptm_node_matcher(node1, node2):
     """
-    Implements matching logic for PTMs
+    Returns True iff node1 and node2 should be considered equal. This means
+    they are both either marked as PTM_atom, or not. If they both are PTM
+    atoms, the elements need to match, and otherwise, the atomnames must
+    match.
     """
-    def __init__(self, *args, **kwargs):
-        super().__init__(*args, **kwargs)
-
-    # G1 >= G2; G1 is the found residue; G2 the PTM reference
-    def semantic_feasibility(self, node1, node2):
-        """
-        Returns True iff node1 and node2 should be considered equal. This means
-        they are both either marked as PTM_atom, or not. If they both are PTM
-        atoms, the elements need to match, and otherwise, the atomnames must
-        match.
-        """
-        node1 = self.G1.nodes[node1]
-        node2 = self.G2.nodes[node2]
-        if node1.get('PTM_atom', False) == node2['PTM_atom']:
-            if node2['PTM_atom']:
-                # elements must match
-                return node1['element'] == node2['element']
-            else:
-                # atomnames must match
-                return node1['atomname'] == node2['atomname']
+    if node1.get('PTM_atom', False) == node2['PTM_atom']:
+        if node2['PTM_atom']:
+            # elements must match
+            return node1['element'] == node2['element']
         else:
-            return False
+            # atomnames must match
+            return node1['atomname'] == node2['atomname']
+    else:
+        return False
 
 
 def find_ptm_atoms(molecule):
@@ -139,7 +129,7 @@ def identify_ptms(residue, residue_ptms, known_ptms):
         As returned by ``find_PTM_atoms``, but only those relevant for
         ``residue``.
 
-    known_PTMs : collections.abc.Sequence[tuple[networkx.Graph, PTMGraphMatcher]]
+    known_PTMs : collections.abc.Sequence[tuple[networkx.Graph, networkx.isomorphism.GraphMatcher]]
         The nodes in the graph must have the `PTM_atom` attribute (True or
         False). It should be True for atoms that are not part of the PTM
         itself, but describe where it is attached to the molecule.
@@ -219,12 +209,12 @@ def allowed_ptms(residue, res_ptms, known_ptms):
 
     Yields
     ------
-    tuple[networkx.Graph, PTMGraphMatcher]
+    tuple[networkx.Graph, networkx.isomorphism.GraphMatcher]
         All graphs in known_ptms which are subgraphs of residue.
     """
     # TODO: filter by element count first
     for ptm in known_ptms:
-        ptm_graph_matcher = PTMGraphMatcher(residue, ptm)
+        ptm_graph_matcher = nx.isomorphism.GraphMatcher(residue, ptm, node_match=ptm_node_matcher)
         if ptm_graph_matcher.subgraph_is_isomorphic():
             yield ptm, ptm_graph_matcher
 

vermouth/processors/do_links.py

@@ -24,18 +24,6 @@
 from .processor import Processor
 
 
-class LinkGraphMatcher(nx.isomorphism.isomorphvf2.GraphMatcher):
-    def __init__(self, *args, **kwargs):
-        super().__init__(*args, **kwargs)
-
-    def semantic_feasibility(self, node1_name, node2_name):
-        # TODO: implement (partial) wildcards
-        # Node2 is the link
-        node1 = self.G1.nodes[node1_name]
-        node2 = self.G2.nodes[node2_name]
-        return _atoms_match(node1, node2)
-
-
 def _atoms_match(node1, node2):
     return attributes_match(node1, node2, ignore_keys=('order', 'replace'))
 
@@ -225,7 +213,7 @@ def match_link(molecule, link):
     if not attributes_match(molecule.meta, link.molecule_meta):
         return
 
-    GM = LinkGraphMatcher(molecule, link)
+    GM = nx.isomorphism.GraphMatcher(molecule, link, node_match=_atoms_match)
 
     raw_matches = GM.subgraph_isomorphisms_iter()
     for raw_match in raw_matches:

vermouth/processors/do_mapping.py

@@ -158,11 +158,14 @@ def build_graph_mapping_collection(from_ff, to_ff, mappings):
     return graph_mapping_collection
 
 
+# We can't inherit from nx.isomorphism.GraphMatcher to override
+# `semantic_feasibility`. That implementation will clobber this one's method.
 class MappingGraphMatcher(nx.isomorphism.isomorphvf2.GraphMatcher):
     def __init__(self, *args, edge_match=None, node_match=None, **kwargs):
         self.edge_match = edge_match
         self.node_match = node_match
         super().__init__(*args, **kwargs)
+        self.G1_adj = self.G1.adj
 
     def semantic_feasibility(self, G1_node, G2_node):
         """
@@ -182,7 +185,7 @@ def semantic_feasibility(self, G1_node, G2_node):
             core_1 = self.core_1
             edge_match = self.edge_match
 
-            for neighbor in self.G1.adj[G1_node]:
+            for neighbor in self.G1_adj[G1_node]:
                 # G1_node is not in core_1, so we must handle R_self separately
                 if neighbor == G1_node:
                     if not edge_match(G1_node, G1_node, G2_node, G2_node):
@@ -341,14 +344,14 @@ def do_mapping(molecule, mappings, to_ff, attribute_keep=()):
                            " particles in your output and/or erroneous bonds."
                            " {}. They've end up in the following output atoms:"
                            " {}.",
-                           [format_atom_string(molecule.nodes[idx]) for idx in shared_atoms],
-                           [format_atom_string(graph_out.nodes[idx]) for idx in shared_out_atoms],
+                           [format_atom_string(molecule.nodes[idx], atomid=idx) for idx in shared_atoms],
+                           [format_atom_string(graph_out.nodes[idx], atomid=idx) for idxs in shared_out_atoms for idx in idxs],
                            type='inconsistent-data')
 
     # Sanity check the results
     if any(v > 1 for v in blocks_per_atom.values()):
         LOGGER.warning('These atoms are covered by multiple blocks. This is a '
-                       'bad idea: {}', {format_atom_string(molecule.nodes[k]): v
+                       'bad idea: {}', {format_atom_string(molecule.nodes[k], atomid=k): v
                                         for k, v in blocks_per_atom.items() if v > 1},
                        type='inconsistent-data')
     uncovered_atoms = set(molecule.nodes.keys()) - set(mol_to_out.keys())

vermouth/tests/test_ptm_detection.py

@@ -185,7 +185,7 @@ def test_identify_ptms(known_ptm_graphs, atoms, edges, expected):
     molecule = make_molecule(atoms, edges)
 
     ptms = canmod.find_ptm_atoms(molecule)
-    known_ptms = [(ptm_graph, canmod.PTMGraphMatcher(molecule, ptm_graph))
+    known_ptms = [(ptm_graph, nx.isomorphism.GraphMatcher(molecule, ptm_graph, node_match=canmod.ptm_node_matcher))
                   for ptm_graph in known_ptm_graphs]
 
     found = canmod.identify_ptms(molecule, ptms, known_ptms)

vermouth/utils.py

@@ -34,7 +34,9 @@ def _distance(point_1, point_2):
     distance = _distance
 
 
-def format_atom_string(node):
+def format_atom_string(node, **kwargs):
+    node = node.copy()
+    node.update(kwargs)
     return '{atomid}{chain}-{resname}{resid}:{atomname}'.format(**node)