From 33d5ce6b2e03ceaf2144078334df4026e7d31bc0 Mon Sep 17 00:00:00 2001 From: Aliaksandr Dziarkach <18146690+AliaksandrDziarkach@users.noreply.github.com> Date: Mon, 23 Oct 2023 14:29:24 +0300 Subject: [PATCH 1/5] #1360 Wrong ordering of atoms in atom list Fix code, add UT --- .../integration/ref/formats/smarts.py.out | 6 +++ api/tests/integration/tests/formats/smarts.py | 6 +++ core/indigo-core/molecule/query_molecule.h | 4 +- .../molecule/src/query_molecule.cpp | 51 ++++++++++--------- 4 files changed, 41 insertions(+), 26 deletions(-) diff --git a/api/tests/integration/ref/formats/smarts.py.out b/api/tests/integration/ref/formats/smarts.py.out index f335aca6ec..8aa62bd961 100644 --- a/api/tests/integration/ref/formats/smarts.py.out +++ b/api/tests/integration/ref/formats/smarts.py.out @@ -61,3 +61,9 @@ smarts loaded OK [!#6!#7!#8] is ok. smarts_in==smarts_out [!#6!#7!#8] is ok. json_in==json_out [!#6!#7!#8] is ok. expected string found in json +[!#40!#79!#30]-[#6]-[#6] is ok. smarts_in==smarts_out +[!#40!#79!#30]-[#6]-[#6] is ok. json_in==json_out +[!#40!#79!#30]-[#6]-[#6] is ok. expected string found in json +[#40,#79,#30]-[#6]-[#6] is ok. smarts_in==smarts_out +[#40,#79,#30]-[#6]-[#6] is ok. json_in==json_out +[#40,#79,#30]-[#6]-[#6] is ok. expected string found in json diff --git a/api/tests/integration/tests/formats/smarts.py b/api/tests/integration/tests/formats/smarts.py index f1cb3aa367..2fb718cbac 100755 --- a/api/tests/integration/tests/formats/smarts.py +++ b/api/tests/integration/tests/formats/smarts.py @@ -184,3 +184,9 @@ def test_smarts_load_save_through_ket(smarts_in, expected_str): "[!#6!#7!#8]", '"atoms":[{"type":"atom-list","notList":true,"elements":["C","N","O"],"location":[0.0,0.0,0.0]}]', ) +smarts = "[!#40!#79!#30]-[#6]-[#6]" +expected = '"atoms":[{"type":"atom-list","notList":true,"elements":["Zr","Au","Zn"],' +test_smarts_load_save_through_ket(smarts, expected) +smarts = "[#40,#79,#30]-[#6]-[#6]" +expected = '"atoms":[{"type":"atom-list","elements":["Zr","Au","Zn"],' +test_smarts_load_save_through_ket(smarts, expected) \ No newline at end of file diff --git a/core/indigo-core/molecule/query_molecule.h b/core/indigo-core/molecule/query_molecule.h index dc3ca2b60a..b87bdfb1c5 100644 --- a/core/indigo-core/molecule/query_molecule.h +++ b/core/indigo-core/molecule/query_molecule.h @@ -430,8 +430,8 @@ namespace indigo void _removeBonds(const Array& indices) override; using AtomList = std::pair>; - static bool _isAtomListOr(const Atom* pqa, std::set& list); - static bool _isAtomOrListAndProps(const Atom* pqa, std::set& list, bool& neg, std::map& properties); + static bool _isAtomListOr(const Atom* pqa, std::vector& list); + static bool _isAtomOrListAndProps(const Atom* pqa, std::vector& list, bool& neg, std::map& properties); static bool _isAtomList(const Atom* qa, AtomList list); Array _min_h; diff --git a/core/indigo-core/molecule/src/query_molecule.cpp b/core/indigo-core/molecule/src/query_molecule.cpp index 8c2989bd75..d2eb87f2ac 100644 --- a/core/indigo-core/molecule/src/query_molecule.cpp +++ b/core/indigo-core/molecule/src/query_molecule.cpp @@ -2339,20 +2339,20 @@ QueryMolecule::Atom* QueryMolecule::stripKnownAttrs(QueryMolecule::Atom& qa) return qd == NULL ? &qa : qd; } -bool QueryMolecule::_isAtomListOr(const Atom* p_query_atom, std::set& list) +bool QueryMolecule::_isAtomListOr(const Atom* p_query_atom, std::vector& list) { // Check if p_query_atom atom list like or(a1,a2,a3, or(a4,a5,a6), a7) if (!p_query_atom) return false; if (p_query_atom->type != OP_OR) return false; - std::set collected; + std::vector collected; for (auto i = 0; i < p_query_atom->children.size(); i++) { Atom* p_query_atom_child = const_cast(p_query_atom)->child(i); if (p_query_atom_child->type == ATOM_NUMBER && p_query_atom_child->value_max == p_query_atom_child->value_max) { - collected.insert(p_query_atom_child->value_min); + collected.emplace_back(p_query_atom_child->value_min); } else if (p_query_atom_child->type == OP_OR) { @@ -2364,18 +2364,18 @@ bool QueryMolecule::_isAtomListOr(const Atom* p_query_atom, std::set& list) } if (collected.size() < 1) return false; - list.insert(collected.begin(), collected.end()); + list = collected; return true; } -bool QueryMolecule::_isAtomOrListAndProps(const Atom* p_query_atom, std::set& list, bool& neg, std::map& properties) +bool QueryMolecule::_isAtomOrListAndProps(const Atom* p_query_atom, std::vector& list, bool& neg, std::map& properties) { // Check if p_query_atom contains only atom or atom list and atom properties connected by "and" // atom list is positive i.e. or(a1,a2,a3,or(a4,a5),a6) or negative // negative list like is set of op_not(atom_number) or op_not(positevi list), this set connected by "and" if (!p_query_atom) return false; - std::set collected; + std::vector collected; std::map collected_properties; if (p_query_atom->type != OP_AND) { @@ -2388,7 +2388,7 @@ bool QueryMolecule::_isAtomOrListAndProps(const Atom* p_query_atom, std::settype == ATOM_NUMBER && p_query_atom_child->value_min == p_query_atom_child->value_max) { - list.insert(p_query_atom_child->value_min); + list.emplace_back(p_query_atom_child->value_min); neg = is_neg; return true; } @@ -2400,7 +2400,8 @@ bool QueryMolecule::_isAtomOrListAndProps(const Atom* p_query_atom, std::set 0 && is_neg != neg) // allowed only one list type in set - positive or negative return false; neg = is_neg; - list.insert(collected.begin(), collected.end()); + for (auto item : collected) + list.emplace_back(item); + collected.clear(); properties.insert(collected_properties.begin(), collected_properties.end()); + collected_properties.clear(); } else return false; @@ -2428,7 +2432,7 @@ bool QueryMolecule::_isAtomOrListAndProps(const Atom* p_query_atom, std::set& list, std::map& properties) { - std::set atom_list; + std::vector atom_list; std::map atom_pros; bool negative = false; QueryMolecule::Atom& qa = qm.getAtom(aid); @@ -2436,31 +2440,30 @@ int QueryMolecule::parseQueryAtomSmarts(QueryMolecule& qm, int aid, std::vector< return QUERY_ATOM_AH; if (_isAtomOrListAndProps(&qa, atom_list, negative, atom_pros)) { - for (auto atom : atom_list) - list.emplace_back(atom); - std::sort(list.begin(), list.end()); + list = atom_list; + std::sort(atom_list.begin(), atom_list.end()); properties.insert(atom_pros.begin(), atom_pros.end()); if (negative) { - if (atom_list.size() == 1 && atom_list.count(ELEM_H) > 0) + if (atom_list.size() == 1 && atom_list[0] == ELEM_H) return QUERY_ATOM_A; // !H - else if (list == std::vector{ELEM_H, ELEM_C}) + else if (atom_list == std::vector{ELEM_H, ELEM_C}) return QUERY_ATOM_Q; - else if (list == std::vector{ELEM_C}) + else if (atom_list == std::vector{ELEM_C}) return QUERY_ATOM_QH; - else if (list == std::vector{ELEM_H, ELEM_He, ELEM_C, ELEM_N, ELEM_O, ELEM_F, ELEM_Ne, ELEM_P, ELEM_S, ELEM_Cl, ELEM_Ar, ELEM_Se, ELEM_Br, - ELEM_Kr, ELEM_I, ELEM_Xe, ELEM_At, ELEM_Rn}) + else if (atom_list == std::vector{ELEM_H, ELEM_He, ELEM_C, ELEM_N, ELEM_O, ELEM_F, ELEM_Ne, ELEM_P, ELEM_S, ELEM_Cl, ELEM_Ar, ELEM_Se, ELEM_Br, + ELEM_Kr, ELEM_I, ELEM_Xe, ELEM_At, ELEM_Rn}) return QUERY_ATOM_M; - else if (list == std::vector{ELEM_He, ELEM_C, ELEM_N, ELEM_O, ELEM_F, ELEM_Ne, ELEM_P, ELEM_S, ELEM_Cl, ELEM_Ar, ELEM_Se, ELEM_Br, ELEM_Kr, - ELEM_I, ELEM_Xe, ELEM_At, ELEM_Rn}) + else if (atom_list == std::vector{ELEM_He, ELEM_C, ELEM_N, ELEM_O, ELEM_F, ELEM_Ne, ELEM_P, ELEM_S, ELEM_Cl, ELEM_Ar, ELEM_Se, ELEM_Br, + ELEM_Kr, ELEM_I, ELEM_Xe, ELEM_At, ELEM_Rn}) return QUERY_ATOM_MH; } else { - if (list == std::vector{ELEM_F, ELEM_Cl, ELEM_Br, ELEM_I, ELEM_At}) + if (atom_list == std::vector{ELEM_F, ELEM_Cl, ELEM_Br, ELEM_I, ELEM_At}) return QUERY_ATOM_X; - else if (list == std::vector{ELEM_H, ELEM_F, ELEM_Cl, ELEM_Br, ELEM_I, ELEM_At}) + else if (atom_list == std::vector{ELEM_H, ELEM_F, ELEM_Cl, ELEM_Br, ELEM_I, ELEM_At}) return QUERY_ATOM_XH; } if (negative) @@ -2469,9 +2472,9 @@ int QueryMolecule::parseQueryAtomSmarts(QueryMolecule& qm, int aid, std::vector< } else { - if (list.size() == 0) + if (atom_list.size() == 0) return QUERY_ATOM_A; - else if (list.size() == 1) + else if (atom_list.size() == 1) return QUERY_ATOM_SINGLE; else return QUERY_ATOM_LIST; From 242da27b9ca96581a7be772b9506d21ef0da045f Mon Sep 17 00:00:00 2001 From: Aliaksandr Dziarkach <18146690+AliaksandrDziarkach@users.noreply.github.com> Date: Mon, 23 Oct 2023 14:34:47 +0300 Subject: [PATCH 2/5] Update smarts.py.out --- api/tests/integration/ref/formats/smarts.py.out | 2 ++ 1 file changed, 2 insertions(+) diff --git a/api/tests/integration/ref/formats/smarts.py.out b/api/tests/integration/ref/formats/smarts.py.out index 8aa62bd961..6e6e3933e5 100644 --- a/api/tests/integration/ref/formats/smarts.py.out +++ b/api/tests/integration/ref/formats/smarts.py.out @@ -61,6 +61,8 @@ smarts loaded OK [!#6!#7!#8] is ok. smarts_in==smarts_out [!#6!#7!#8] is ok. json_in==json_out [!#6!#7!#8] is ok. expected string found in json +#1355 Error appeared at save query molecule with RSite as smarts +Ok expected smarts generated [!#40!#79!#30]-[#6]-[#6] is ok. smarts_in==smarts_out [!#40!#79!#30]-[#6]-[#6] is ok. json_in==json_out [!#40!#79!#30]-[#6]-[#6] is ok. expected string found in json From dffa23e9dfc0a4df2f10fc80fdd5e005775bfa97 Mon Sep 17 00:00:00 2001 From: Aliaksandr Dziarkach <18146690+AliaksandrDziarkach@users.noreply.github.com> Date: Mon, 23 Oct 2023 15:41:22 +0300 Subject: [PATCH 3/5] Update smarts.py --- api/tests/integration/tests/formats/smarts.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/api/tests/integration/tests/formats/smarts.py b/api/tests/integration/tests/formats/smarts.py index 3b4af47dd7..f1315b1505 100755 --- a/api/tests/integration/tests/formats/smarts.py +++ b/api/tests/integration/tests/formats/smarts.py @@ -200,6 +200,8 @@ def test_smarts_load_save_through_ket(smarts_in, expected_str): expected = '"atoms":[{"type":"atom-list","notList":true,"elements":["Zr","Au","Zn"],' test_smarts_load_save_through_ket(smarts, expected) smarts = "[#40,#79,#30]-[#6]-[#6]" -expected = '"atoms":[{"type":"atom-list","elements":["Zr","Au","Zn"],' +expected = ( + '"atoms":[{"type":"atom-list","elements":["Zr","Au","Zn"],' +) test_smarts_load_save_through_ket(smarts, expected) From a8ca90ee02b357d11c3c4de047e62a9a9d4974ba Mon Sep 17 00:00:00 2001 From: Aliaksandr Dziarkach <18146690+AliaksandrDziarkach@users.noreply.github.com> Date: Mon, 23 Oct 2023 15:45:05 +0300 Subject: [PATCH 4/5] Update smarts.py --- api/tests/integration/tests/formats/smarts.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/api/tests/integration/tests/formats/smarts.py b/api/tests/integration/tests/formats/smarts.py index f1315b1505..c0bbcc5a73 100755 --- a/api/tests/integration/tests/formats/smarts.py +++ b/api/tests/integration/tests/formats/smarts.py @@ -197,11 +197,11 @@ def test_smarts_load_save_through_ket(smarts_in, expected_str): "Fail. Expected smarts is\n%s\nbut generated\n%s" % (expected, smarts) ) smarts = "[!#40!#79!#30]-[#6]-[#6]" -expected = '"atoms":[{"type":"atom-list","notList":true,"elements":["Zr","Au","Zn"],' -test_smarts_load_save_through_ket(smarts, expected) -smarts = "[#40,#79,#30]-[#6]-[#6]" expected = ( - '"atoms":[{"type":"atom-list","elements":["Zr","Au","Zn"],' + '"atoms":[{"type":"atom-list","notList":true,"elements":["Zr","Au","Zn"],' ) test_smarts_load_save_through_ket(smarts, expected) +smarts = "[#40,#79,#30]-[#6]-[#6]" +expected = '"atoms":[{"type":"atom-list","elements":["Zr","Au","Zn"],' +test_smarts_load_save_through_ket(smarts, expected) From b97dc3c2af3beace66dade9cfb72705613133777 Mon Sep 17 00:00:00 2001 From: Aliaksandr Dziarkach <18146690+AliaksandrDziarkach@users.noreply.github.com> Date: Mon, 23 Oct 2023 15:49:22 +0300 Subject: [PATCH 5/5] Update smarts.py --- api/tests/integration/tests/formats/smarts.py | 1 - 1 file changed, 1 deletion(-) diff --git a/api/tests/integration/tests/formats/smarts.py b/api/tests/integration/tests/formats/smarts.py index c0bbcc5a73..9eb0b8538c 100755 --- a/api/tests/integration/tests/formats/smarts.py +++ b/api/tests/integration/tests/formats/smarts.py @@ -204,4 +204,3 @@ def test_smarts_load_save_through_ket(smarts_in, expected_str): smarts = "[#40,#79,#30]-[#6]-[#6]" expected = '"atoms":[{"type":"atom-list","elements":["Zr","Au","Zn"],' test_smarts_load_save_through_ket(smarts, expected) -