Enable additional parameters in prepareAndDrawMolecule() and expose them to CFFI/MinimalLib (#5731)

* - extend prepareAndDrawMolecule() with missing optional parameters already supported by prepareMolForDrawing()
- enable useMolBlockWedging, wedgeBonds, addChiralHs, forceCoords, wavyBonds in CFFI/MinimalLib
- add relevant CFFI and JS unit tests

* Replace AllProps with a smaller subset

* make sure we pickle properties Python-side

* changes in response to review

Co-authored-by: Tosco, Paolo <paolo.tosco@novartis.com>
This commit is contained in:
Paolo Tosco
2022-11-08 11:50:09 +01:00
committed by GitHub
parent 776441c064
commit 739b417dfe
8 changed files with 165 additions and 77 deletions

View File

@@ -92,9 +92,11 @@ void prepareAndDrawMolecule(MolDraw2D &drawer, const ROMol &mol,
const std::map<int, DrawColour> *highlight_atom_map,
const std::map<int, DrawColour> *highlight_bond_map,
const std::map<int, double> *highlight_radii,
int confId, bool kekulize) {
int confId, bool kekulize, bool addChiralHs,
bool wedgeBonds, bool forceCoords, bool wavyBonds) {
RWMol cpy(mol);
prepareMolForDrawing(cpy, kekulize);
prepareMolForDrawing(cpy, kekulize, addChiralHs, wedgeBonds, forceCoords,
wavyBonds);
// having done the prepare, we don't want to do it again in drawMolecule.
bool old_prep_mol = drawer.drawOptions().prepareMolsBeforeDrawing;
drawer.drawOptions().prepareMolsBeforeDrawing = false;

View File

@@ -70,7 +70,8 @@ RDKIT_MOLDRAW2D_EXPORT void prepareAndDrawMolecule(
const std::map<int, DrawColour> *highlight_atom_map = nullptr,
const std::map<int, DrawColour> *highlight_bond_map = nullptr,
const std::map<int, double> *highlight_radii = nullptr, int confId = -1,
bool kekulize = true);
bool kekulize = true, bool addChiralHs = true, bool wedgeBonds = true,
bool forceCoords = false, bool wavyBonds = false);
RDKIT_MOLDRAW2D_EXPORT void updateDrawerParamsFromJSON(MolDraw2D &drawer,
const char *json);

View File

@@ -236,7 +236,55 @@ void test_svg() {
free(svg);
free(pkl);
pkl = NULL;
pkl = get_mol(
"\n\
MJ201100 \n\
\n\
9 10 0 0 1 0 0 0 0 0999 V2000\n\
1.4885 -4.5513 0.0000 O 0 0 0 0 0 0 0 0 0 0 0 0\n\
2.0405 -3.9382 0.0000 C 0 0 0 0 0 0 0 0 0 0 0 0\n\
2.8610 -4.0244 0.0000 C 0 0 0 0 0 0 0 0 0 0 0 0\n\
3.1965 -3.2707 0.0000 C 0 0 0 0 0 0 0 0 0 0 0 0\n\
3.0250 -2.4637 0.0000 C 0 0 0 0 0 0 0 0 0 0 0 0\n\
2.2045 -2.3775 0.0000 C 0 0 0 0 0 0 0 0 0 0 0 0\n\
1.7920 -1.6630 0.0000 N 0 0 0 0 0 0 0 0 0 0 0 0\n\
1.8690 -3.1311 0.0000 C 0 0 0 0 0 0 0 0 0 0 0 0\n\
2.5834 -2.7186 0.0000 C 0 0 0 0 0 0 0 0 0 0 0 0\n\
2 1 1 1 0 0 0\n\
2 3 1 0 0 0 0\n\
4 3 1 0 0 0 0\n\
4 5 1 0 0 0 0\n\
6 5 1 0 0 0 0\n\
6 7 1 1 0 0 0\n\
6 8 1 0 0 0 0\n\
8 9 1 1 0 0 0\n\
8 2 1 0 0 0 0\n\
4 9 1 1 0 0 0\n\
M END\n",
&pkl_size, "");
assert(pkl);
assert(pkl_size > 0);
char *svg1 = get_svg(pkl, pkl_size, "{\"width\":350,\"height\":300}");
assert(strstr(svg1, "width='350px'"));
assert(strstr(svg1, "height='300px'"));
assert(strstr(svg1, "</svg>"));
assert(strstr(svg1, "atom-8"));
assert(strstr(svg1, "atom-9"));
assert(strstr(svg1, "atom-10"));
char *svg2 = get_svg(
pkl, pkl_size,
"{\"width\":350,\"height\":300,\"useMolBlockWedging\":true,\"wedgeBonds\":false,\"addChiralHs\":false}");
assert(strstr(svg2, "width='350px'"));
assert(strstr(svg2, "height='300px'"));
assert(strstr(svg2, "</svg>"));
assert(strstr(svg2, "atom-8"));
assert(!strstr(svg2, "atom-9"));
assert(!strstr(svg2, "atom-10"));
free(svg1);
free(svg2);
free(pkl);
printf(" done\n");
printf("--------------------------\n");
}

View File

@@ -67,6 +67,20 @@
#pragma GCC diagnostic pop
#endif
#endif
#define GET_JSON_VALUE(doc, key, type) \
const auto key##It = doc.FindMember(#key); \
if (key##It != doc.MemberEnd()) { \
if (!key##It->value.Is##type()) { \
return "JSON contains '" #key "' field, but its type is not '" #type \
"'"; \
} \
key = key##It->value.Get##type(); \
}
#define GET_JSON_VALUE_WITH_DEFAULT(doc, key, type, defaultValue) \
GET_JSON_VALUE(doc, key, type) else key = defaultValue;
namespace rj = rapidjson;
namespace RDKit {
@@ -323,7 +337,9 @@ std::string parse_highlight_colors(const rj::Document &doc,
std::string process_details(rj::Document &doc, const std::string &details,
int &width, int &height, int &offsetx, int &offsety,
std::string &legend, std::vector<int> &atomIds,
std::vector<int> &bondIds, bool &kekulize) {
std::vector<int> &bondIds, bool &kekulize,
bool &addChiralHs, bool &wedgeBonds,
bool &forceCoords, bool &wavyBonds) {
doc.Parse(details.c_str());
if (!doc.IsObject()) {
return "Invalid JSON";
@@ -339,55 +355,16 @@ std::string process_details(rj::Document &doc, const std::string &details,
return problems;
}
const auto widthIt = doc.FindMember("width");
if (widthIt != doc.MemberEnd()) {
if (!widthIt->value.IsInt()) {
return "JSON contains 'width' field, but it is not an int";
}
width = widthIt->value.GetInt();
}
const auto heightIt = doc.FindMember("height");
if (heightIt != doc.MemberEnd()) {
if (!heightIt->value.IsInt()) {
return "JSON contains 'height' field, but it is not an int";
}
height = heightIt->value.GetInt();
}
const auto offsetxIt = doc.FindMember("offsetx");
if (offsetxIt != doc.MemberEnd()) {
if (!offsetxIt->value.IsInt()) {
return "JSON contains 'offsetx' field, but it is not an int";
}
offsetx = offsetxIt->value.GetInt();
}
const auto offsetyIt = doc.FindMember("offsety");
if (offsetyIt != doc.MemberEnd()) {
if (!offsetyIt->value.IsInt()) {
return "JSON contains 'offsety' field, but it is not an int";
}
offsety = offsetyIt->value.GetInt();
}
const auto legendIt = doc.FindMember("legend");
if (legendIt != doc.MemberEnd()) {
if (!legendIt->value.IsString()) {
return "JSON contains 'legend' field, but it is not a string";
}
legend = legendIt->value.GetString();
}
const auto kekulizeIt = doc.FindMember("kekulize");
if (kekulizeIt != doc.MemberEnd()) {
if (!kekulizeIt->value.IsBool()) {
return "JSON contains 'kekulize' field, but it is not a bool";
}
kekulize = kekulizeIt->value.GetBool();
} else {
kekulize = true;
}
GET_JSON_VALUE(doc, width, Int)
GET_JSON_VALUE(doc, height, Int)
GET_JSON_VALUE(doc, offsetx, Int)
GET_JSON_VALUE(doc, offsety, Int)
GET_JSON_VALUE(doc, legend, String)
GET_JSON_VALUE_WITH_DEFAULT(doc, kekulize, Bool, true)
GET_JSON_VALUE_WITH_DEFAULT(doc, addChiralHs, Bool, true)
GET_JSON_VALUE_WITH_DEFAULT(doc, wedgeBonds, Bool, true)
GET_JSON_VALUE_WITH_DEFAULT(doc, forceCoords, Bool, false)
GET_JSON_VALUE_WITH_DEFAULT(doc, wavyBonds, Bool, false)
return "";
}
@@ -398,11 +375,13 @@ std::string process_mol_details(const std::string &details, int &width,
std::vector<int> &bondIds,
std::map<int, DrawColour> &atomMap,
std::map<int, DrawColour> &bondMap,
std::map<int, double> &radiiMap,
bool &kekulize) {
std::map<int, double> &radiiMap, bool &kekulize,
bool &addChiralHs, bool &wedgeBonds,
bool &forceCoords, bool &wavyBonds) {
rj::Document doc;
auto problems = process_details(doc, details, width, height, offsetx, offsety,
legend, atomIds, bondIds, kekulize);
auto problems = process_details(
doc, details, width, height, offsetx, offsety, legend, atomIds, bondIds,
kekulize, addChiralHs, wedgeBonds, forceCoords, wavyBonds);
if (!problems.empty()) {
return problems;
}
@@ -440,20 +419,17 @@ std::string process_rxn_details(
std::vector<int> &bondIds, bool &kekulize, bool &highlightByReactant,
std::vector<DrawColour> &highlightColorsReactants) {
rj::Document doc;
auto problems = process_details(doc, details, width, height, offsetx, offsety,
legend, atomIds, bondIds, kekulize);
bool addChiralHs;
bool wedgeBonds;
bool forceCoords;
bool wavyBonds;
auto problems = process_details(
doc, details, width, height, offsetx, offsety, legend, atomIds, bondIds,
kekulize, addChiralHs, wedgeBonds, forceCoords, wavyBonds);
if (!problems.empty()) {
return problems;
}
auto highlightByReactantIt = doc.FindMember("highlightByReactant");
if (highlightByReactantIt != doc.MemberEnd()) {
if (!highlightByReactantIt->value.IsBool()) {
return "JSON contains 'highlightByReactant' field, but it is not a bool";
}
highlightByReactant = highlightByReactantIt->value.GetBool();
} else {
highlightByReactant = false;
}
GET_JSON_VALUE_WITH_DEFAULT(doc, highlightByReactant, Bool, false)
auto highlightColorsReactantsIt = doc.FindMember("highlightColorsReactants");
if (highlightColorsReactantsIt != doc.MemberEnd()) {
if (!highlightColorsReactantsIt->value.IsArray()) {
@@ -510,10 +486,15 @@ std::string mol_to_svg(const ROMol &m, int w, int h,
int offsetx = 0;
int offsety = 0;
bool kekulize = true;
bool addChiralHs = true;
bool wedgeBonds = true;
bool forceCoords = false;
bool wavyBonds = false;
if (!details.empty()) {
problems =
process_mol_details(details, w, h, offsetx, offsety, legend, atomIds,
bondIds, atomMap, bondMap, radiiMap, kekulize);
bondIds, atomMap, bondMap, radiiMap, kekulize,
addChiralHs, wedgeBonds, forceCoords, wavyBonds);
if (!problems.empty()) {
return problems;
}
@@ -528,7 +509,8 @@ std::string mol_to_svg(const ROMol &m, int w, int h,
atomMap.empty() ? nullptr : &atomMap,
bondMap.empty() ? nullptr : &bondMap,
radiiMap.empty() ? nullptr : &radiiMap,
-1, kekulize);
-1, kekulize, addChiralHs, wedgeBonds,
forceCoords, wavyBonds);
drawer.finishDrawing();
return drawer.getDrawingText();

View File

@@ -25,7 +25,8 @@ extern std::string process_mol_details(
int &offsety, std::string &legend, std::vector<int> &atomIds,
std::vector<int> &bondIds, std::map<int, DrawColour> &atomMap,
std::map<int, DrawColour> &bondMap, std::map<int, double> &radiiMap,
bool &kekulize);
bool &kekulize, bool &addChiralHs, bool &wedgeBonds, bool &forceCoords,
bool &wavyBonds);
extern std::string process_rxn_details(
const std::string &details, int &width, int &height, int &offsetx,
int &offsety, std::string &legend, std::vector<int> &atomIds,
@@ -77,10 +78,15 @@ std::string draw_to_canvas_with_highlights(JSMol &self, emscripten::val canvas,
int offsetx = 0;
int offsety = 0;
bool kekulize = true;
bool addChiralHs = true;
bool wedgeBonds = true;
bool forceCoords = false;
bool wavyBonds = false;
if (!details.empty()) {
auto problems = MinimalLib::process_mol_details(
details, w, h, offsetx, offsety, legend, atomIds, bondIds, atomMap,
bondMap, radiiMap, kekulize);
bondMap, radiiMap, kekulize, addChiralHs, wedgeBonds, forceCoords,
wavyBonds);
if (!problems.empty()) {
return problems;
}
@@ -96,7 +102,8 @@ std::string draw_to_canvas_with_highlights(JSMol &self, emscripten::val canvas,
*d2d, *self.d_mol, legend, &atomIds, &bondIds,
atomMap.empty() ? nullptr : &atomMap,
bondMap.empty() ? nullptr : &bondMap,
radiiMap.empty() ? nullptr : &radiiMap, -1, kekulize);
radiiMap.empty() ? nullptr : &radiiMap, -1, kekulize, addChiralHs,
wedgeBonds, forceCoords, wavyBonds);
return "";
}

View File

@@ -145,7 +145,8 @@ std::string JSMol::get_pickle() const {
return "";
}
std::string pickle;
MolPickler::pickleMol(*d_mol, pickle);
MolPickler::pickleMol(*d_mol, pickle,
PicklerOps::AllProps ^ PicklerOps::ComputedProps);
return pickle;
}
@@ -709,4 +710,4 @@ void prefer_coordgen(bool useCoordGen) {
void use_legacy_stereo_perception(bool value) {
Chirality::setUseLegacyStereoPerception(value);
}
}

View File

@@ -1018,6 +1018,50 @@ function test_highlights() {
assert(svg.includes('</svg>'));
}
function test_add_chiral_hs() {
var mol = RDKitModule.get_mol(`
MJ201100
9 10 0 0 1 0 0 0 0 0999 V2000
1.4885 -4.5513 0.0000 O 0 0 0 0 0 0 0 0 0 0 0 0
2.0405 -3.9382 0.0000 C 0 0 0 0 0 0 0 0 0 0 0 0
2.8610 -4.0244 0.0000 C 0 0 0 0 0 0 0 0 0 0 0 0
3.1965 -3.2707 0.0000 C 0 0 0 0 0 0 0 0 0 0 0 0
3.0250 -2.4637 0.0000 C 0 0 0 0 0 0 0 0 0 0 0 0
2.2045 -2.3775 0.0000 C 0 0 0 0 0 0 0 0 0 0 0 0
1.7920 -1.6630 0.0000 N 0 0 0 0 0 0 0 0 0 0 0 0
1.8690 -3.1311 0.0000 C 0 0 0 0 0 0 0 0 0 0 0 0
2.5834 -2.7186 0.0000 C 0 0 0 0 0 0 0 0 0 0 0 0
2 1 1 1 0 0 0
2 3 1 0 0 0 0
4 3 1 0 0 0 0
4 5 1 0 0 0 0
6 5 1 0 0 0 0
6 7 1 1 0 0 0
6 8 1 0 0 0 0
8 9 1 1 0 0 0
8 2 1 0 0 0 0
4 9 1 1 0 0 0
M END
`);
var svg1 = mol.get_svg_with_highlights(JSON.stringify({width: 350, height: 300}));
assert(svg1.includes("width='350px'"));
assert(svg1.includes("height='300px'"));
assert(svg1.includes("</svg>"));
assert(svg1.includes("atom-8"));
assert(svg1.includes("atom-9"));
assert(svg1.includes("atom-10"));
var svg2 = mol.get_svg_with_highlights(JSON.stringify({
width: 350, height: 300, useMolBlockWedging: true, wedgeBonds: false, addChiralHs: false
}));
assert(svg2.includes("width='350px'"));
assert(svg2.includes("height='300px'"));
assert(svg2.includes("</svg>"));
assert(svg2.includes("atom-8"));
assert(!svg2.includes("atom-9"));
assert(!svg2.includes("atom-10"));
}
initRDKitModule().then(function(instance) {
var done = {};
const waitAllTestsFinished = () => {
@@ -1062,6 +1106,7 @@ initRDKitModule().then(function(instance) {
test_legacy_stereochem();
test_prop();
test_highlights();
test_add_chiral_hs();
waitAllTestsFinished().then(() =>
console.log("Tests finished successfully")
);

View File

@@ -259,7 +259,9 @@ def xmlToNewline(xmlblock):
def toDataMol(mol):
return "pkl_" + base64.b64encode(mol.ToBinary()).decode("utf-8")
return "pkl_" + base64.b64encode(mol.ToBinary(
Chem.PropertyPickleOptions.AllProps ^ Chem.PropertyPickleOptions.ComputedProps
)).decode("utf-8")
def _dashLower(m):