From a7515e9b96b9497e23f3424187c30350980fe09e Mon Sep 17 00:00:00 2001
From: Ben Webb <ben@salilab.org>
Date: Tue, 17 Nov 2020 12:44:15 -0800
Subject: [PATCH] Add tests and better docs for add_atom_types()

Document the edge cases for adding CHARMM atom
types. Add tests for reassigning existing types.
---
 .../atom/include/charmm_segment_topology.h    |  4 +++
 .../test/expensive_test_charmm_topology.py    | 27 +++++++++++++++++++
 2 files changed, 31 insertions(+)

diff --git a/modules/atom/include/charmm_segment_topology.h b/modules/atom/include/charmm_segment_topology.h
index 3d957d5897..b578da31ea 100644
--- a/modules/atom/include/charmm_segment_topology.h
+++ b/modules/atom/include/charmm_segment_topology.h
@@ -117,6 +117,10 @@ class IMPATOMEXPORT CHARMMTopology : public IMP::Object {
 
   //! Add CHARMM atom types to the given Hierarchy using this topology.
   /** The primary sequence of the Hierarchy must match that of the topology.
+      A warning will be printed for any atoms that cannot be found in the
+      topology (and no CHARMM atom type will be assigned). Any atoms that
+      already have a CHARMM atom type and that are present in the topology
+      will have that type reassigned.
       \see CHARMMAtom.
    */
   void add_atom_types(Hierarchy hierarchy) const;
diff --git a/modules/atom/test/expensive_test_charmm_topology.py b/modules/atom/test/expensive_test_charmm_topology.py
index dd430e9a7d..2305c22cc2 100644
--- a/modules/atom/test/expensive_test_charmm_topology.py
+++ b/modules/atom/test/expensive_test_charmm_topology.py
@@ -605,6 +605,33 @@ def test_get_remove_untyped_atoms(self):
         atoms = IMP.atom.get_by_type(pdb, IMP.atom.ATOM_TYPE)
         self.assertEqual(len(atoms), 7)
 
+    def test_warn_previously_typed(self):
+        """Test warning about previously-typed CHARMM atoms"""
+        m = IMP.Model()
+        pdb = IMP.atom.read_pdb(
+            self.get_input_file_name('charmm_type_test.pdb'), m)
+        ff = IMP.atom.CHARMMParameters(IMP.atom.get_data_path("top.lib"),
+                                       IMP.atom.get_data_path("par.lib"))
+        atoms = IMP.atom.get_by_type(pdb, IMP.atom.ATOM_TYPE)
+        # Set manual CHARMM atom types
+        dum, = [a for a in atoms
+                if IMP.atom.Atom(a).get_atom_type().get_string() == 'DUM']
+        ca1, ca2, = [a for a in atoms
+                if IMP.atom.Atom(a).get_atom_type().get_string() == 'CA']
+        IMP.atom.CHARMMAtom.setup_particle(dum, "OD")
+        IMP.atom.CHARMMAtom.setup_particle(ca1, "N")
+
+        topology = ff.create_topology(pdb)
+        topology.apply_default_patches()
+        topology.add_atom_types(pdb)
+
+        # dum should have kept its original type, OD
+        self.assertEqual(IMP.atom.CHARMMAtom(dum).get_charmm_type(), 'OD')
+        # ca1 should have been assigned the correct type
+        self.assertEqual(IMP.atom.CHARMMAtom(ca1).get_charmm_type(), 'CT1')
+        untyped = IMP.atom.get_charmm_untyped_atoms(pdb)
+        self.assertEqual(len(untyped), 0)
+
     def test_add_missing_atoms(self):
         """Test adding missing atoms"""
         m = IMP.Model()