Decouple parmed from the foyer atomtyper

[umesh-timalsina]

This issue has been discussed at length in #386. Specifically this PR is intended to address #386 (comment) and with #382 underway. I think we can safely commit to removing third party packages for atomtyping engine from foyer.

Checklist:

• Remove parmed reference from atom_typer.py
• Remove atom references in smarts_graph.py
• Build a separate utility that subclasses nx.Graph, which has to have certain attributes needed for atom_typing? Discussion needed.

Based on the third point a utility class could look like the following:

import networkx as nx
from parmed import Structure


class AtomData:
    def __init__(self, index, name, atomic_number, element, bond_partners, **kwargs):
        self.index = index
        self.name = name
        self.atomic_number = atomic_number
        self.element = element
        self.bond_partners = bond_partners
        for key, value in kwargs.items():
            setattr(self, key, value)


class TopologyGraph(nx.Graph):
    def __init__(self, *args, **kwargs):
        super(TopologyGraph, self).__init__(*args, **kwargs)

    def add_atom(self, index, name, atomic_number, element, bond_partners, **kwargs):
        atom_data = AtomData(
            index, name, atomic_number, element, bond_partners, **kwargs
        )
        self.add_node(index, atom_data=atom_data)

    def add_bond(self, atom_1_index, atom_2_index):
        self.add_edge(atom_1_index, atom_2_index)

    def atoms(self, data=False):
        if data:
            for idx, data in self.nodes(data=data):
                yield idx, data["atom_data"]
        else:
            for idx in self.nodes(data=data):
                yield idx

    @classmethod
    def from_parmed(cls, structure: Structure) -> nx.Graph:
        """Return a networkx graph with relevant attributes from a parmed Structure"""
        topology_graph = cls()
        for atom in structure.atoms:
            topology_graph.add_atom(
                name=atom.name,
                index=atom.idx,
                atomic_number=atom.atomic_number,
                element=atom.element,
                bond_partners=[bonded_atom.idx for bonded_atom in atom.bond_partners],
            )

        for bond in structure.bonds:
            topology_graph.add_bond(bond.atom1.idx, bond.atom2.idx)
        return topology_graph

[umesh-timalsina]

This PR is now complete for initial round of reviews.

[lgtm-com]

This pull request introduces 2 alerts when merging f969c13 into e80015c - view on LGTM.com

new alerts:

• 2 for Unused import

[justinGilmer]

Reviewed in tandem with @umesh-timalsina and @daico007 . LGTM!

[daico007]

LGTM!

[rsdefever]

I don't think this is what you want here. The atom.element attribute is actually the atomic number. The atom.element_name attribute is the two-character element code.

[umesh-timalsina]

Agreed.

[rsdefever]

What do we do for the atomic_number of non-element atom types?

[umesh-timalsina]

For non_element atom types, this attribute is not used.

[umesh-timalsina]

But I see your point. Will change this to be optional

[rsdefever]

This line is why I was asking about how non-element atom types handle atomic_number.

[codecov]

Codecov Report

Merging #389 (b0b120b) into master (8953210) will increase coverage by 0.34%.
The diff coverage is 95.87%.

❗ Current head b0b120b differs from pull request most recent head bd2c296. Consider uploading reports for the commit bd2c296 to get more accurate results

@@            Coverage Diff             @@
##           master     #389      +/-   ##
==========================================
+ Coverage   85.08%   85.42%   +0.34%     
==========================================
  Files          15       16       +1     
  Lines        1448     1496      +48     
==========================================
+ Hits         1232     1278      +46     
- Misses        216      218       +2