-
Notifications
You must be signed in to change notification settings - Fork 128
#1256 - Inability to save a structure with R-Group in the SDF file format #3763
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 10 commits
c664b73
f7e8a14
d77d18e
e689983
7132642
a505a94
728c7c7
22bdad7
fc02cac
8df7810
a44e5ea
42df4a7
aefff60
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,6 +19,10 @@ | |
| #include "indigo_savers.h" | ||
|
|
||
| #include <ctime> | ||
| #include <memory> | ||
|
|
||
| #include "indigo_molecule.h" | ||
| #include "indigo_reaction.h" | ||
|
|
||
| #include "base_cpp/output.h" | ||
| #include "base_cpp/scanner.h" | ||
|
|
@@ -353,6 +357,61 @@ void IndigoSdfSaver::saveMonomerLibrary(const MonomerTemplateLibrary& monomers_l | |
| _output.flush(); | ||
| } | ||
|
|
||
| // Counts atoms that live outside the main molecule graph. R-groups are stored | ||
| // independently of the molecule's connected components: an R-group fragment is a | ||
| // separate sub-molecule whose atoms are not vertices of the main graph, so the | ||
| // component iteration never visits them on its own. Returns the number of such | ||
| // out-of-graph atoms (currently those held by R-group fragments). | ||
| static int countOutOfGraphAtoms(BaseMolecule& mol) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it seems like this function just counts number of all atoms in r-groups
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. logic updated |
||
| { | ||
| int count = 0; | ||
| for (int i = 1; i <= mol.rgroups.getRGroupCount(); i++) | ||
| { | ||
| RGroup& rgroup = mol.rgroups.getRGroup(i); | ||
| for (int j = rgroup.fragments.begin(); j != rgroup.fragments.end(); j = rgroup.fragments.next(j)) | ||
| count += rgroup.fragments[j]->vertexCount(); | ||
| } | ||
| return count; | ||
| } | ||
|
|
||
| void IndigoSdfSaver::appendFragments(Output& output, IndigoObject& object) | ||
| { | ||
| if (IndigoBaseReaction::is(object)) | ||
| { | ||
| // A reaction is split into its constituent molecules. | ||
| IndigoReactionIter fragments(object.getBaseReaction(), IndigoReactionIter::MOLECULES); | ||
| while (fragments.hasNext()) | ||
| { | ||
| std::unique_ptr<IndigoObject> fragment(fragments.next()); | ||
| std::unique_ptr<IndigoObject> clone(fragment->clone()); | ||
| IndigoSdfSaver::append(output, *clone); | ||
| } | ||
| return; | ||
| } | ||
|
|
||
| // A molecule is split into the connected components of its main graph. Each | ||
| // component clone also carries the molecule's R-groups, so once at least one | ||
| // component is written the out-of-graph (R-group) atoms are written with it. | ||
| BaseMolecule& mol = object.getBaseMolecule(); | ||
| int written_atoms = 0; | ||
| IndigoComponentsIter fragments(mol); | ||
| while (fragments.hasNext()) | ||
| { | ||
| std::unique_ptr<IndigoObject> fragment(fragments.next()); | ||
| std::unique_ptr<IndigoObject> clone(fragment->clone()); | ||
|
NikolaiBalabanov marked this conversation as resolved.
Outdated
|
||
| written_atoms += clone->getBaseMolecule().vertexCount(); | ||
| IndigoSdfSaver::append(output, *clone); | ||
| } | ||
|
|
||
| // R-groups and the connected components are independent: an R-group may or may | ||
| // not be pulled into a component record. If, after writing every component, | ||
| // atoms remain that were not part of any written fragment (e.g. a free R-group | ||
| // whose atoms live outside the main graph), emit the whole molecule as one more | ||
| // record so the owning structure is preserved instead of being lost. (#1256) | ||
| if (written_atoms == 0 && countOutOfGraphAtoms(mol) > 0) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it will make more sense to write each r-group independent if it is does not belong to any existing structure. also, it makes sense to consider not only written_atoms=0 case (e.g. if structure contains r-group assigned to atoms, and r-group outside the graph
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. logic updated |
||
| IndigoSdfSaver::append(output, object); | ||
| } | ||
|
|
||
| CEXPORT int indigoSdfAppend(int output, int molecule) | ||
| { | ||
| INDIGO_BEGIN | ||
|
|
@@ -365,6 +424,20 @@ CEXPORT int indigoSdfAppend(int output, int molecule) | |
| INDIGO_END(-1); | ||
| } | ||
|
|
||
| CEXPORT const char* indigoGetFragmentSdf(int item) | ||
| { | ||
| INDIGO_BEGIN | ||
| { | ||
| IndigoObject& obj = self.getObject(item); | ||
| auto& tmp = self.getThreadTmpData(); | ||
| ArrayOutput out(tmp.string); | ||
| IndigoSdfSaver::appendFragments(out, obj); | ||
| tmp.string.push(0); | ||
| return tmp.string.ptr(); | ||
| } | ||
| INDIGO_END(0); | ||
| } | ||
|
|
||
| // | ||
| // IndigoSmilesSaver | ||
| // | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -304,6 +304,20 @@ def cml(self): | |
|
|
||
| return IndigoLib.checkResultString(self._lib().indigoCml(self.id)) | ||
|
|
||
| def getFragmentSdf(self): | ||
| """Structure method returns the structure as a string in SDF format, | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Non-blocker: Let's either add comment in all wrappers or take it off from all places.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it is depended from file. indigo_object.py has comments in this style. |
||
| splitting it into fragments: connected components for a molecule or | ||
| constituent molecules for a reaction. Each fragment is written as a | ||
| separate SDF record. | ||
|
|
||
| Returns: | ||
| str: SDF string | ||
| """ | ||
|
|
||
| return IndigoLib.checkResultString( | ||
| self._lib().indigoGetFragmentSdf(self.id) | ||
| ) | ||
|
|
||
| def saveCdxml(self, filename): | ||
| """Molecule method saves the structure into a CDXML file | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| *** KET to SDF *** | ||
| 1256-ketR20-from2815.sdf:SUCCEED |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| import os | ||
| import sys | ||
|
|
||
| sys.path.append( | ||
| os.path.normpath( | ||
| os.path.join(os.path.abspath(__file__), "..", "..", "..", "common") | ||
| ) | ||
| ) | ||
| from common.util import compare_diff | ||
| from env_indigo import * # noqa | ||
|
|
||
| indigo = Indigo() | ||
| indigo.setOption("json-saving-pretty", True) | ||
| indigo.setOption("molfile-saving-skip-date", True) | ||
| indigo.setOption("ignore-stereochemistry-errors", True) | ||
|
|
||
| print("*** KET to SDF ***") | ||
|
|
||
| root = joinPathPy("molecules/", __file__) | ||
| ref_path = joinPathPy("ref/", __file__) | ||
|
|
||
| files = ["1256-ketR20-from2815"] | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. add tests with
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you please provide specific files in ket format to fill the gap?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Some similar data has been added
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. On top of the cases already requested above, please also add a case with ≥2 disconnected normal components plus one attached R-group (e.g. a simple salt)
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you please provide specific files in ket format to fill the gap?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Some similar data has been added |
||
|
|
||
| files.sort() | ||
| for filename in files: | ||
| try: | ||
| ket = indigo.loadMoleculeFromFile( | ||
| os.path.join(root, filename + ".ket") | ||
| ) | ||
| except IndigoException: | ||
| ket = indigo.loadQueryMoleculeFromFile( | ||
| os.path.join(root, filename + ".ket") | ||
| ) | ||
|
|
||
| sdf = ket.getFragmentSdf() | ||
| compare_diff(ref_path, filename + ".sdf", sdf) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor: Consider align naming to support consistency across application.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
naming updated