Bugfix/#2045 Export of unresolved monomer to everything except KET and IDT should cause specific error message (but it doesn't) #2806
Conversation
api/c/tests/unit/tests/formats.cpp
Outdated
| try | ||
| { | ||
| int molecule = -1; | ||
| molecule = indigoLoadKetDocumentFromFile(dataPath("molecules/basic/unknown_monomers.ket").c_str()); |
There was a problem hiding this comment.
Please use indigoLoadMoleculeFromFile - indigoCanonicalSmarts doesn't support KetDocument yet.
api/c/tests/unit/tests/formats.cpp
Outdated
| try | ||
| { | ||
| int molecule = -1; | ||
| molecule = indigoLoadKetDocumentFromFile(dataPath("molecules/basic/unknown_monomers.ket").c_str()); |
There was a problem hiding this comment.
Please use indigoLoadMoleculeFromFile - indigoSmarts doesn't support KetDocument yet.
| } | ||
| catch (const std::exception& e) | ||
| { | ||
| ASSERT_STREQ("core: <KetDocument> can not be converted to SMARTS", e.what()); |
There was a problem hiding this comment.
This message looks wrong.
Looks like no exception thrown.
Could you please rewrite these tests like
catch (const std::exception& e)
{
ASSERT_STREQ(msg, e.what());
return;
}
ASSERT(false);
just to be sure that exception thrown
There was a problem hiding this comment.
The exception is thrown in void IndigoSmilesSaver::generateSmarts(IndigoObject& obj, Array<char>& out_buffer) in indigo_severs.cpp.
...
loader_tmp.loadQueryReaction(qreaction);
saver.saveQueryReaction(qreaction);
}
}
else
throw IndigoError("%s can not be converted to SMARTS", obj.debugInfo());
out_buffer.push(0);
In this case, do I still need to write code in the way you suggest?
There was a problem hiding this comment.
Yes. Beacuse after indigoLoadMoleculeFromFile molecule not a KetDocument - but error message contains this type wich is strange.
There was a problem hiding this comment.
I've checked. Yes, it is not throwing an exception.
In void IndigoSmilesSaver::generateSmarts(IndigoObject& obj, Array<char>& out_buffer) in
...
{
Array<char> mol_out_buffer;
ArrayOutput mol_output(mol_out_buffer);
MolfileSaver saver_tmp(mol_output);
saver_tmp.saveMolecule(mol.asMolecule());
mol_out_buffer.push(0);
BufferScanner sc(mol_out_buffer);
MolfileLoader loader_tmp(sc);
QueryMolecule qmol;
loader_tmp.loadQueryMolecule(qmol);
saver.saveQueryMolecule(qmol);
}
...
-
The first one called is
void MolfileSaver::saveMolecule(Molecule& mol)==>void MolfileSaver::saveMolecule(Molecule& mol)==>void MolfileSaver::_validate(BaseMolecule& bmol) -
The second one called is
void SmilesSaver::saveQueryMolecule(QueryMolecule& mol)==>void SmilesSaver::_saveMolecule()==>void SmilesSaver::_validate(BaseMolecule& bmol)
At the end of those calls bool BaseMolecule::getUnresolvedTemplatesList(BaseMolecule& bmol, std::string& unresolved) is called
The first and the second ones have different behavior:
- unresolved.size() returns 0
- does not even enter the if-body:
if (!bmol.isQueryMolecule())
{
if (bmol.tgroups.getTGroupCount())
{
for (auto tgidx = bmol.tgroups.begin(); tgidx != bmol.tgroups.end(); tgidx = bmol.tgroups.next(tgidx))
{
auto& tg = bmol.tgroups.getTGroup(tgidx);
if (tg.unresolved && tg.tgroup_alias.size())
{
if (unresolved.size())
unresolved += ',';
unresolved += tg.tgroup_alias.ptr();
}
}
}
}
Could you tell, please, which behavior is expected and which one is not?
Maybe you have any other suggestions to fix the bug due to my lack of understanding of how the things are working.
There was a problem hiding this comment.
Please remove if (!bmol.isQueryMolecule()) - looks like this is something from first versions without query molecules support.
There was a problem hiding this comment.
getUnresolvedTemplatesList should return correct list of unresolved templates.
If it return wrong list - thois is a bug.
There was a problem hiding this comment.
According to getUnresolvedTemplatesList code it could return empty list if uresolved has no alias.
Pleas add hasUnresolvedTemplates wich will return true if at least one unresolved template exists.
And use it instead of getUnresolvedTemplatesList
Fix code. Add UT.
Generic request
#1234 – issue nameOptional