#1358 Error while loading [!#6,!#7,!#8] smarts (#1361)

epam · Oct 20, 2023 · f9a072a · f9a072a
1 parent 1951ec4
commit f9a072a
Show file tree

Hide file tree

Showing 5 changed files with 55 additions and 32 deletions.
diff --git a/api/tests/integration/ref/formats/smarts.py.out b/api/tests/integration/ref/formats/smarts.py.out
@@ -55,3 +55,9 @@ CC[C+5]CCCCC
 [#6]-;!@[#6] is ok. expected string found in json
 #1292 test smarts c:1-2:c(:c:c:c:c:1)-[#6](=[#8])-[#6;X4]-[#6]-2=[#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
+[!#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
diff --git a/api/tests/integration/tests/formats/ref/ket_with_custom_query.ket b/api/tests/integration/tests/formats/ref/ket_with_custom_query.ket
@@ -50,7 +50,7 @@
                 ]
             },
             {
-                "label": "C",
+                "label": "",
                 "location": [
                     7.203820705413818,
                     -7.050033092498779,

diff --git a/api/tests/integration/tests/formats/smarts.py b/api/tests/integration/tests/formats/smarts.py
@@ -177,3 +177,10 @@ def test_smarts_load_save_through_ket(smarts_in, expected_str):
 print("#1292 test smarts c:1-2:c(:c:c:c:c:1)-[#6](=[#8])-[#6;X4]-[#6]-2=[#8]")
 indigo.loadSmarts("c:1-2:c(:c:c:c:c:1)-[#6](=[#8])-[#6;X4]-[#6]-2=[#8]")
 print("smarts loaded OK")
+test_smarts_load_save_through_ket(
+    "[!#6,!#7,!#8]", '"queryProperties":{"customQuery":"!#6,!#7,!#8"}}]'
+)
+test_smarts_load_save_through_ket(
+    "[!#6!#7!#8]",
+    '"atoms":[{"type":"atom-list","notList":true,"elements":["C","N","O"],"location":[0.0,0.0,0.0]}]',
+)
diff --git a/core/indigo-core/molecule/src/molecule_json_saver.cpp b/core/indigo-core/molecule/src/molecule_json_saver.cpp
@@ -734,45 +734,52 @@ void MoleculeJsonSaver::saveAtoms(BaseMolecule& mol, JsonWriter& writer)
                     }
                 }
             }
-            else if (_pqmol && query_atom_type != QueryMolecule::QUERY_ATOM_UNKNOWN)
+            else if (_pqmol)
             {
-                if (query_atom_type == QueryMolecule::QUERY_ATOM_LIST || query_atom_type == QueryMolecule::QUERY_ATOM_NOTLIST)
-                {
-                    is_qatom_list = true;
-                    writer.Key("type");
-                    writer.String("atom-list");
-                    if (query_atom_type == QueryMolecule::QUERY_ATOM_NOTLIST)
-                    {
-                        writer.Key("notList");
-                        writer.Bool(true);
-                    }
-                    writer.Key("elements");
-                    writer.StartArray();
-                    for (auto atom : atoms)
-                        writer.String(Element::toString(atom));
-                    writer.EndArray();
-                }
-                else if (query_atom_type == QueryMolecule::QUERY_ATOM_SINGLE)
+                if (query_atom_type != QueryMolecule::QUERY_ATOM_UNKNOWN)
                 {
-                    anum = *atoms.begin();
-                    buf.readString(Element::toString(anum), true);
-                    if (anum == ELEM_H && query_atom_properties.count(QueryMolecule::ATOM_ISOTOPE) > 0)
+                    if (query_atom_type == QueryMolecule::QUERY_ATOM_LIST || query_atom_type == QueryMolecule::QUERY_ATOM_NOTLIST)
                     {
-                        int isotope = query_atom_properties[QueryMolecule::ATOM_ISOTOPE]->value_min;
-                        if (isotope == DEUTERIUM)
+                        is_qatom_list = true;
+                        writer.Key("type");
+                        writer.String("atom-list");
+                        if (query_atom_type == QueryMolecule::QUERY_ATOM_NOTLIST)
                         {
-                            buf.clear();
-                            buf.appendString("D", true);
+                            writer.Key("notList");
+                            writer.Bool(true);
                         }
-                        else if (isotope == TRITIUM)
+                        writer.Key("elements");
+                        writer.StartArray();
+                        for (auto atom : atoms)
+                            writer.String(Element::toString(atom));
+                        writer.EndArray();
+                    }
+                    else if (query_atom_type == QueryMolecule::QUERY_ATOM_SINGLE)
+                    {
+                        anum = *atoms.begin();
+                        buf.readString(Element::toString(anum), true);
+                        if (anum == ELEM_H && query_atom_properties.count(QueryMolecule::ATOM_ISOTOPE) > 0)
                         {
-                            buf.clear();
-                            buf.appendString("T", true);
+                            int isotope = query_atom_properties[QueryMolecule::ATOM_ISOTOPE]->value_min;
+                            if (isotope == DEUTERIUM)
+                            {
+                                buf.clear();
+                                buf.appendString("D", true);
+                            }
+                            else if (isotope == TRITIUM)
+                            {
+                                buf.clear();
+                                buf.appendString("T", true);
+                            }
                         }
                     }
+                    else
+                        QueryMolecule::getQueryAtomLabel(query_atom_type, buf);
+                }
+                else // query_atom_type == QueryMolecule::QUERY_ATOM_UNKNOWN
+                {
+                    buf.push(0); // Set label to empty string
                 }
-                else
-                    QueryMolecule::getQueryAtomLabel(query_atom_type, buf);
             }
 
             if (!is_qatom_list)

diff --git a/core/indigo-core/molecule/src/query_molecule.cpp b/core/indigo-core/molecule/src/query_molecule.cpp
@@ -1864,6 +1864,7 @@ void QueryMolecule::Atom::copy(const Atom& other)
     type = other.type;
     value_max = other.value_max;
     value_min = other.value_min;
+    artificial = other.artificial;
 
     fragment.reset(nullptr);
     if (other.fragment.get() != 0)
@@ -2408,6 +2409,8 @@ bool QueryMolecule::_isAtomOrListAndProps(const Atom* p_query_atom, std::set<int
     for (auto i = 0; i < p_query_atom->children.size(); i++)
     {
         Atom* p_query_atom_child = const_cast<Atom*>(p_query_atom)->child(i);
+        if (p_query_atom_child->type == OP_NOT && p_query_atom_child->artificial)
+            continue; // !H added by SmilesLoader::_forbidHydrogens
         bool is_neg = false;
         if (_isAtomOrListAndProps(p_query_atom_child, collected, is_neg, collected_properties))
         {
@@ -2427,7 +2430,7 @@ int QueryMolecule::parseQueryAtomSmarts(QueryMolecule& qm, int aid, std::vector<
 {
     std::set<int> atom_list;
     std::map<int, const Atom*> atom_pros;
-    bool negative;
+    bool negative = false;
     QueryMolecule::Atom& qa = qm.getAtom(aid);
     if (qa.type == QueryMolecule::OP_NONE)
         return QUERY_ATOM_AH;