From 051a5f926007faa79ab04daeb5b281ea3fc15b2a Mon Sep 17 00:00:00 2001
From: Gabriel Studer <gabriel.studer@unibas.ch>
Date: Tue, 9 Apr 2024 08:24:33 +0200
Subject: [PATCH] remove quack_mode property from IOProfile
The property didn't hold what it promised in the documentation and
had undesired sideeffects and collided with the fault_tolerant property
when loading PDB files.
---
.../mm/ethanol_example_using_topology.py | 2 +-
modules/io/doc/profile.rst | 18 +++--------
modules/io/pymod/__init__.py | 14 +++-----
modules/io/pymod/export_pdb_io.cc | 4 +--
modules/io/src/mol/io_profile.hh | 10 +++---
modules/io/src/mol/mmcif_reader.cc | 4 +--
modules/io/src/mol/pdb_reader.cc | 32 +++++++++----------
modules/io/tests/test_io_pdb.cc | 6 ++--
modules/io/tests/test_io_pdb.py | 12 +++----
modules/mol/mm/src/observer.cc | 2 +-
10 files changed, 41 insertions(+), 63 deletions(-)
diff --git a/examples/code_fragments/mm/ethanol_example_using_topology.py b/examples/code_fragments/mm/ethanol_example_using_topology.py
index 553ef8860..2b060fcae 100644
--- a/examples/code_fragments/mm/ethanol_example_using_topology.py
+++ b/examples/code_fragments/mm/ethanol_example_using_topology.py
@@ -44,7 +44,7 @@ class Anim(QtCore.QTimer):
#create topology by only defining masses
prof = io.IOProfile(dialect='PDB', fault_tolerant=False,
- quack_mode=False, processor=conop.HeuristicProcessor())
+ processor=conop.HeuristicProcessor())
ent = io.LoadPDB('ethanol.pdb', profile=prof)
masses = [12.011,12.011,15.999,1.008,1.008,1.008,1.008,1.008,1.008]
diff --git a/modules/io/doc/profile.rst b/modules/io/doc/profile.rst
index 9ab8a395c..0dd990867 100644
--- a/modules/io/doc/profile.rst
+++ b/modules/io/doc/profile.rst
@@ -52,7 +52,7 @@ STRICT
.. code-block:: python
- IOProfile(dialect='PDB', fault_tolerant=False, quack_mode=False,
+ IOProfile(dialect='PDB', fault_tolerant=False,
processor=conop.RuleBasedProcessor(conop.GetDefaultLib()))
SLOPPY:
@@ -61,7 +61,7 @@ SLOPPY:
.. code-block:: python
- IOProfile(dialect='PDB', fault_tolerant=True, quack_mode=True,
+ IOProfile(dialect='PDB', fault_tolerant=True,
processor=conop.RuleBasedProcessor(conop.GetDefaultLib()))
CHARMM:
@@ -71,7 +71,7 @@ CHARMM:
.. code-block:: python
- IOProfile(dialect='CHARMM', fault_tolerant=True, quack_mode=False,
+ IOProfile(dialect='CHARMM', fault_tolerant=True,
processor=conop.RuleBasedProcessor(conop.GetDefaultLib()))
.. note::
@@ -92,7 +92,7 @@ CHARMM:
The IOProfile Class
--------------------------------------------------------------------------------
-.. class:: IOProfile(dialect='PDB', quack_mode=False, fault_tolerant=False,\
+.. class:: IOProfile(dialect='PDB', fault_tolerant=False,\
join_spread_atom_records=False, no_hetatms=False,\
calpha_only=False, read_conect=False, processor=None)
@@ -108,16 +108,6 @@ The IOProfile Class
support for chain names with length up to 4 characters (column 72-76) and
increase the size of the residue name to 4 residues.
- .. attribute:: quack_mode
-
- :type: bool
-
- Read/write property. When quack_mode is enabled, the chemical class for
- unknown residues is guessed based on its atoms and connectivity. Turn this
- on if you are working with non-standard conforming PDB files and are
- experiencing problems with the rendering of the backbone trace and/or see
- peptidic residues with unknown chemical classes.
-
.. attribute:: fault_tolerant
:type: bool
diff --git a/modules/io/pymod/__init__.py b/modules/io/pymod/__init__.py
index 91d2593a2..c29edd1f9 100644
--- a/modules/io/pymod/__init__.py
+++ b/modules/io/pymod/__init__.py
@@ -30,11 +30,11 @@ class IOProfiles:
else:
processor = conop.HeuristicProcessor()
self['STRICT'] = IOProfile(dialect='PDB', fault_tolerant=False,
- quack_mode=False, processor=processor.Copy())
+ processor=processor.Copy())
self['SLOPPY'] = IOProfile(dialect='PDB', fault_tolerant=True,
- quack_mode=True, processor=processor.Copy())
+ processor=processor.Copy())
self['CHARMM'] = IOProfile(dialect='CHARMM', fault_tolerant=True,
- quack_mode=False, processor=processor.Copy())
+ processor=processor.Copy())
self['DEFAULT'] = 'STRICT'
def __getitem__(self, key):
@@ -80,7 +80,7 @@ def _override(val1, val2):
return val1
def LoadPDB(filename, restrict_chains="", no_hetatms=None,
- fault_tolerant=None, load_multi=False, quack_mode=None,
+ fault_tolerant=None, load_multi=False,
join_spread_atom_records=None, calpha_only=None,
profile='DEFAULT', remote=False, remote_repo='pdb',
dialect=None, seqres=False, bond_feasibility_check=None,
@@ -112,11 +112,6 @@ def LoadPDB(filename, restrict_chains="", no_hetatms=None,
multi-PDB files.
:type load_multi: :class:`bool`
- :param quack_mode: Guess the chemical class for unknown residues based on its
- atoms and connectivity. If set, overrides the value of
- :attr:`IOProfile.quack_mode`.
- :type quack_mode: :class:`bool`
-
:param join_spread_atom_records: If set, overrides the value of
:attr:`IOProfile.join_spread_atom_records`.
:type join_spread_atom_records: :class:`bool`
@@ -206,7 +201,6 @@ def LoadPDB(filename, restrict_chains="", no_hetatms=None,
prof.calpha_only=_override(prof.calpha_only, calpha_only)
prof.no_hetatms=_override(prof.no_hetatms, no_hetatms)
prof.dialect=_override(prof.dialect, dialect)
- prof.quack_mode=_override(prof.quack_mode, quack_mode)
prof.read_conect=_override(prof.read_conect, read_conect)
if prof.processor:
prof.processor.check_bond_feasibility=_override(prof.processor.check_bond_feasibility,
diff --git a/modules/io/pymod/export_pdb_io.cc b/modules/io/pymod/export_pdb_io.cc
index cad44b855..7e0926352 100644
--- a/modules/io/pymod/export_pdb_io.cc
+++ b/modules/io/pymod/export_pdb_io.cc
@@ -45,9 +45,8 @@ void remove_profiles() {
void export_pdb_io()
{
class_<IOProfile>("IOProfile",
- init<String,bool,bool,bool,bool,bool,bool,
+ init<String,bool,bool,bool,bool,bool,
conop::ProcessorPtr>((arg("dialect")="PDB",
- arg("quack_mode")=false,
arg("fault_tolerant")=false,
arg("join_spread_atom_records")=false,
arg("no_hetatms")=false,
@@ -57,7 +56,6 @@ void export_pdb_io()
.def(init<const IOProfile&>())
.def_readwrite("dialect", &IOProfile::dialect)
.def_readwrite("fault_tolerant", &IOProfile::fault_tolerant)
- .def_readwrite("quack_mode", &IOProfile::quack_mode)
.def_readwrite("no_hetatms", &IOProfile::no_hetatms)
.def_readwrite("calpha_only", &IOProfile::calpha_only)
.def_readwrite("join_spread_atom_records", &IOProfile::join_spread_atom_records)
diff --git a/modules/io/src/mol/io_profile.hh b/modules/io/src/mol/io_profile.hh
index d8f078e39..ac2dee89b 100644
--- a/modules/io/src/mol/io_profile.hh
+++ b/modules/io/src/mol/io_profile.hh
@@ -30,20 +30,19 @@ namespace ost { namespace io {
struct DLLEXPORT IOProfile {
public:
- IOProfile(String d, bool qm, bool ft, bool js, bool nh,
+ IOProfile(String d, bool ft, bool js, bool nh,
bool co, bool rc, conop::ProcessorPtr proc=conop::ProcessorPtr()):
- dialect(d), quack_mode(qm), fault_tolerant(ft), join_spread_atom_records(js),
+ dialect(d), fault_tolerant(ft), join_spread_atom_records(js),
no_hetatms(nh), calpha_only(co), read_conect(rc), processor(proc)
{
}
- IOProfile(): dialect("PDB"), quack_mode(false), fault_tolerant(false),
+ IOProfile(): dialect("PDB"), fault_tolerant(false),
join_spread_atom_records(false), no_hetatms(false),
calpha_only(false), read_conect(false), processor()
{ }
String dialect;
- bool quack_mode;
bool fault_tolerant;
bool join_spread_atom_records;
bool no_hetatms;
@@ -52,7 +51,7 @@ public:
conop::ProcessorPtr processor;
IOProfile Copy()
{
- return IOProfile(dialect, quack_mode, fault_tolerant, join_spread_atom_records,
+ return IOProfile(dialect, fault_tolerant, join_spread_atom_records,
no_hetatms, calpha_only, read_conect,
processor ? processor->Copy() : conop::ProcessorPtr());
}
@@ -62,7 +61,6 @@ public:
inline std::ostream& operator<<(std::ostream& stream, const IOProfile& p)
{
stream << "IOProfile(dialect='" << p.dialect
- << "', quack_mode=" << (p.quack_mode ? "True" : "False") << ", "
<< "join_spread_atom_records=" << (p.join_spread_atom_records ? "True" : "False") << ", "
<< "calpha_only=" << (p.calpha_only ? "True" : "False") << ", "
<< "fault_tolerant=" << (p.fault_tolerant ? "True" : "False") << ", "
diff --git a/modules/io/src/mol/mmcif_reader.cc b/modules/io/src/mol/mmcif_reader.cc
index 27a08f089..2fc3bdb26 100644
--- a/modules/io/src/mol/mmcif_reader.cc
+++ b/modules/io/src/mol/mmcif_reader.cc
@@ -700,10 +700,10 @@ void MMCifReader::ParseAndAddAtom(const std::vector<StringRef>& columns)
}
} else {
mol::AtomHandle atom=curr_residue_.FindAtom(aname);
- if (atom.IsValid() && !profile_.quack_mode) { // unit test
+ if (atom.IsValid()) { // unit test
if (profile_.fault_tolerant) { // unit test
LOG_WARNING("duplicate atom '" << aname << "' in residue "
- << curr_residue_);
+ << curr_residue_ << " only first atom added");
return;
}
throw IOException(this->FormatDiagnostic(STAR_DIAG_ERROR,
diff --git a/modules/io/src/mol/pdb_reader.cc b/modules/io/src/mol/pdb_reader.cc
index 78b6bec4d..af1e5f729 100644
--- a/modules/io/src/mol/pdb_reader.cc
+++ b/modules/io/src/mol/pdb_reader.cc
@@ -844,26 +844,24 @@ void PDBReader::ParseAndAddAtom(const StringRef& line, int line_num,
<< "residue with number " << res_num << " has more than one name.";
throw IOException(ss.str());
}
- if(!profile_.quack_mode) {
- if (!warned_name_mismatch_) {
- if (alt_loc==' ') {
- LOG_WARNING("Residue with number " << res_num << " has more than one name. "
- "Ignoring atoms for everything but the first");
- } else {
- LOG_WARNING("Residue with number " << res_num
- << " contains a microheterogeneity. Everything but atoms for "
- << "the residue '" << curr_residue_.GetName()
- << "' will be ignored");
- }
+ if (!warned_name_mismatch_) {
+ if (alt_loc==' ') {
+ LOG_WARNING("Residue with number " << res_num << " has more than one name. "
+ "Ignoring atoms for everything but the first");
+ } else {
+ LOG_WARNING("Residue with number " << res_num
+ << " contains a microheterogeneity. Everything but atoms for "
+ << "the residue '" << curr_residue_.GetName()
+ << "' will be ignored");
}
- warned_name_mismatch_=true;
- return;
}
+ warned_name_mismatch_=true;
+ return;
}
Real b=temp.first ? temp.second : 0.0;
Real o=occ.first ? occ.second : 1.0;
- if (!profile_.quack_mode && alt_loc!=' ') {
+ if (alt_loc!=' ') {
// Check if there is already a atom with the same name.
mol::AtomHandle me=curr_residue_.FindAtom(aname);
if (me.IsValid()) {
@@ -871,7 +869,7 @@ void PDBReader::ParseAndAddAtom(const StringRef& line, int line_num,
editor.AddAltAtomPos(String(1, alt_loc), me, apos, o, b);
} catch (Error&) {
LOG_INFO("Ignoring atom alt location since there is already an atom "
- "with name " << aname << ", but without an alt loc");
+ "with name " << aname << ", but without an alt loc");
return;
}
return;
@@ -882,10 +880,10 @@ void PDBReader::ParseAndAddAtom(const StringRef& line, int line_num,
}
} else {
mol::AtomHandle atom=curr_residue_.FindAtom(aname);
- if (atom.IsValid() && !profile_.quack_mode) {
+ if (atom.IsValid()) {
if (profile_.fault_tolerant) {
LOG_WARNING("duplicate atom '" << aname << "' in residue "
- << curr_residue_);
+ << curr_residue_ << " only first atom added");
return;
}
throw IOException("duplicate atom '"+aname+"' in residue "+
diff --git a/modules/io/tests/test_io_pdb.cc b/modules/io/tests/test_io_pdb.cc
index ce507a6fe..3ebffd12c 100644
--- a/modules/io/tests/test_io_pdb.cc
+++ b/modules/io/tests/test_io_pdb.cc
@@ -975,7 +975,7 @@ BOOST_AUTO_TEST_CASE(charmm_rname)
{
{
PDBWriter writer(String("testfiles/pdb/charmm_rname-out.pdb"),
- IOProfile("CHARMM", false, false, false, false, false, false));
+ IOProfile("CHARMM", false, false, false, false, false));
mol::EntityHandle ent=mol::CreateEntity();
mol::XCSEditor edi=ent.EditXCS();
@@ -994,7 +994,7 @@ BOOST_AUTO_TEST_CASE(charmm_longcname)
{
{
PDBWriter writer(String("testfiles/pdb/charmm_longcname-out.pdb"),
- IOProfile("CHARMM", false, false, false, false, false, false));
+ IOProfile("CHARMM", false, false, false, false, false));
mol::EntityHandle ent=mol::CreateEntity();
mol::XCSEditor edi=ent.EditXCS();
@@ -1013,7 +1013,7 @@ BOOST_AUTO_TEST_CASE(write_charmm_ter)
{
{
PDBWriter writer(String("testfiles/pdb/charmm_ter-out.pdb"),
- IOProfile("CHARMM", false, false, false, false, false, false));
+ IOProfile("CHARMM", false, false, false, false, false));
mol::EntityHandle ent=mol::CreateEntity();
mol::XCSEditor edi=ent.EditXCS();
diff --git a/modules/io/tests/test_io_pdb.py b/modules/io/tests/test_io_pdb.py
index 7a34502ff..33ee8afc1 100644
--- a/modules/io/tests/test_io_pdb.py
+++ b/modules/io/tests/test_io_pdb.py
@@ -14,12 +14,12 @@ class TestPDB(unittest.TestCase):
def test_properly_assigns_profile_properties(self):
io.profiles['TEST'] = io.IOProfile()
- io.profiles['TEST'].quack_mode = False
- self.assertFalse(io.profiles['TEST'].quack_mode)
- self.assertFalse(io.profiles['TEST'].Copy().quack_mode)
- io.profiles['TEST'].quack_mode = True
- self.assertTrue(io.profiles['TEST'].quack_mode)
- self.assertTrue(io.profiles['TEST'].Copy().quack_mode)
+ io.profiles['TEST'].fault_tolerant = False
+ self.assertFalse(io.profiles['TEST'].fault_tolerant)
+ self.assertFalse(io.profiles['TEST'].Copy().fault_tolerant)
+ io.profiles['TEST'].fault_tolerant = True
+ self.assertTrue(io.profiles['TEST'].fault_tolerant)
+ self.assertTrue(io.profiles['TEST'].Copy().fault_tolerant)
def test_no_bond_feasibility(self):
io.profiles['FEAS_CHECK']=io.IOProfile(processor=conop.HeuristicProcessor(check_bond_feasibility=True))
io.profiles['NO_FEAS_CHECK']=io.IOProfile(processor=conop.HeuristicProcessor(check_bond_feasibility=False))
diff --git a/modules/mol/mm/src/observer.cc b/modules/mol/mm/src/observer.cc
index 0abc555da..90a862083 100644
--- a/modules/mol/mm/src/observer.cc
+++ b/modules/mol/mm/src/observer.cc
@@ -55,7 +55,7 @@ void TrajWriter::Init(boost::shared_ptr<OpenMM::Context> c,
registered_ = true;
context_ = c;
- ost::io::IOProfile profile("CHARMM",false,false,false,false,false,false);
+ ost::io::IOProfile profile("CHARMM",false,false,false,false,false);
ost::io::PDBWriter writer(pdb_filename_, profile);
writer.Write(ent.GetAtomList());
--
GitLab