diff --git a/Code/GraphMol/Wrap/Atom.cpp b/Code/GraphMol/Wrap/Atom.cpp index 7b681df61..7832dbbbf 100644 --- a/Code/GraphMol/Wrap/Atom.cpp +++ b/Code/GraphMol/Wrap/Atom.cpp @@ -42,18 +42,18 @@ void setQuery(QueryAtom *self, const QueryAtom *other) { } template -void AtomSetProp(const Atom *atom, const char *key, const T &val) { +void AtomSetProp(const Atom *atom, const std::string &key, const T &val) { // std::cerr<<"asp: "<setProp(key, val); } -int AtomHasProp(const Atom *atom, const char *key) { +int AtomHasProp(const Atom *atom, const std::string &key) { // std::cerr<<"ahp: "<hasProp(key); return res; } -void AtomClearProp(const Atom *atom, const char *key) { +void AtomClearProp(const Atom *atom, const std::string &key) { if (!atom->hasProp(key)) { return; } diff --git a/Code/GraphMol/Wrap/props.hpp b/Code/GraphMol/Wrap/props.hpp index 058b5eb49..9851c1e58 100644 --- a/Code/GraphMol/Wrap/props.hpp +++ b/Code/GraphMol/Wrap/props.hpp @@ -167,11 +167,11 @@ boost::python::dict GetPropsAsDict(const T &obj, bool includePrivate, } template -T GetProp(const RDOb *ob, const char *key) { +T GetProp(const RDOb *ob, const std::string &key) { T res; try { if (!ob->getPropIfPresent(key, res)) { - PyErr_SetString(PyExc_KeyError, key); + PyErr_SetString(PyExc_KeyError, key.c_str()); throw python::error_already_set(); } return res; @@ -203,7 +203,7 @@ template python::object GetPyProp(const RDOb *obj, const std::string &key, bool autoConvert) { if (!autoConvert) { - return python::object(GetProp(obj, key.c_str())); + return python::object(GetProp(obj, key)); } else { auto &data = obj->getDict().getData(); for (auto &rdvalue : data) { @@ -283,20 +283,20 @@ python::object GetPyProp(const RDOb *obj, const std::string &key, } template -int MolHasProp(const RDOb &mol, const char *key) { +int MolHasProp(const RDOb &mol, const std::string &key) { int res = mol.hasProp(key); // std::cout << "key: " << key << ": " << res << std::endl; return res; } template -void MolSetProp(const RDOb &mol, const char *key, const T &val, +void MolSetProp(const RDOb &mol, const std::string &key, const T &val, bool computed = false) { mol.setProp(key, val, computed); } template -void MolClearProp(const RDOb &mol, const char *key) { +void MolClearProp(const RDOb &mol, const std::string &key) { if (!mol.hasProp(key)) { return; } diff --git a/Code/GraphMol/Wrap/rdmolfiles.cpp b/Code/GraphMol/Wrap/rdmolfiles.cpp index 23a8cf433..8c743827e 100644 --- a/Code/GraphMol/Wrap/rdmolfiles.cpp +++ b/Code/GraphMol/Wrap/rdmolfiles.cpp @@ -99,7 +99,7 @@ ROMol *MolFromSmarts(python::object ismarts, bool mergeHs, } return static_cast(newM); } -ROMol *MolFromTPLFile(const char *filename, bool sanitize = true, +ROMol *MolFromTPLFile(const std::string &filename, bool sanitize = true, bool skipFirstConf = false) { RWMol *newM; try { @@ -126,7 +126,7 @@ ROMol *MolFromTPLBlock(python::object itplBlock, bool sanitize = true, return static_cast(newM); } -ROMol *MolFromMolFileHelper(const char *molFilename, bool sanitize, +ROMol *MolFromMolFileHelper(const std::string &molFilename, bool sanitize, bool removeHs, bool strictParsing) { RWMol *newM = nullptr; try { @@ -156,7 +156,7 @@ ROMol *MolFromMolBlock(python::object imolBlock, bool sanitize, bool removeHs, return static_cast(newM); } -ROMol *MolFromMolFile(const char *molFilename, bool sanitize, bool removeHs, +ROMol *MolFromMolFile(const std::string &molFilename, bool sanitize, bool removeHs, bool strictParsing) { RWMol *newM = nullptr; try { @@ -171,7 +171,7 @@ ROMol *MolFromMolFile(const char *molFilename, bool sanitize, bool removeHs, return static_cast(newM); } -ROMol *MolFromMrvFile(const char *molFilename, bool sanitize, bool removeHs) { +ROMol *MolFromMrvFile(const std::string &molFilename, bool sanitize, bool removeHs) { RWMol *newM = nullptr; try { newM = MrvFileToMol(molFilename, sanitize, removeHs); @@ -226,7 +226,7 @@ ROMol *MolFromSVG(python::object imolBlock, bool sanitize, bool removeHs) { return static_cast(res); } -ROMol *MolFromMol2File(const char *molFilename, bool sanitize = true, +ROMol *MolFromMol2File(const std::string &molFilename, bool sanitize = true, bool removeHs = true, bool cleanupSubstructures = true) { RWMol *newM; try { @@ -255,7 +255,7 @@ ROMol *MolFromMol2Block(std::string mol2Block, bool sanitize = true, return static_cast(newM); } -ROMol *MolFromPDBFile(const char *filename, bool sanitize, bool removeHs, +ROMol *MolFromPDBFile(const std::string &filename, bool sanitize, bool removeHs, unsigned int flavor, bool proximityBonding) { RWMol *newM = nullptr; try { @@ -506,7 +506,7 @@ python::list MolToRandomSmilesHelper(const ROMol &mol, unsigned int numSmiles, return pyres; } -ROMol *MolFromPNGFile(const char *filename, python::object pyParams) { +ROMol *MolFromPNGFile(const std::string &filename, python::object pyParams) { SmilesParserParams params; if (pyParams) { params = python::extract(pyParams); @@ -604,7 +604,7 @@ python::object addMetadataToPNGStringHelper(python::dict pymetadata, return retval; } -python::object MolsFromPNGFile(const char *filename, const std::string &tag, +python::object MolsFromPNGFile(const std::string &filename, const std::string &tag, python::object pyParams) { SmilesParserParams params; if (pyParams) { @@ -645,7 +645,7 @@ python::tuple MolsFromPNGString(python::object png, const std::string &tag, return python::tuple(res); } -python::object MolsFromCDXMLFile(const char *filename, bool sanitize, +python::object MolsFromCDXMLFile(const std::string &filename, bool sanitize, bool removeHs) { std::vector> mols; try { diff --git a/Code/GraphMol/Wrap/rough_test.py b/Code/GraphMol/Wrap/rough_test.py index 356530e8f..283426e83 100644 --- a/Code/GraphMol/Wrap/rough_test.py +++ b/Code/GraphMol/Wrap/rough_test.py @@ -8252,7 +8252,24 @@ M END chain2mol = Chem.SplitMolByPDBChainId(mol) self.assertEqual(len(chain2mol), 1) self.assertIn("", chain2mol) - + + def testGithub4570(self): + # test sending None to get/set prop + m = Chem.Mol() + # these should not seg fault + for func in ["HasProp", "GetProp", "GetIntProp", "GetDoubleProp"]: + try: + getattr(m, func)(None) + except Exception as e: + assert "Python argument types" in str(e), f"{func}: {str(e)}" + + for func, val in [("SetProp", ""), ("SetIntProp", 1), ("SetDoubleProp", 1.0)]: + try: + getattr(m, func)(None, val) + except Exception as e: + assert "Python argument types" in str(e), f"{func}: {str(e)}" + + if __name__ == '__main__': if "RDTESTCASE" in os.environ: suite = unittest.TestSuite()