diff --git a/Code/GraphMol/SmilesParse/SmilesJSONParsers.cpp b/Code/GraphMol/SmilesParse/SmilesJSONParsers.cpp index 1a240b9f5..3e00d464d 100644 --- a/Code/GraphMol/SmilesParse/SmilesJSONParsers.cpp +++ b/Code/GraphMol/SmilesParse/SmilesJSONParsers.cpp @@ -9,6 +9,7 @@ // #define USE_BETTER_ENUMS +#include #include #include #include @@ -49,19 +50,9 @@ void updateCXSmilesFieldsFromJSON(std::uint32_t &cxSmilesFields, std::istringstream ss; ss.str(details_json); boost::property_tree::read_json(ss, pt); - auto cxSmilesFieldsFromJson = - (+SmilesWrite::CXSmilesFields::CX_NONE)._to_integral(); bool haveCXSmilesFields = false; - for (const auto *key : SmilesWrite::CXSmilesFields::_names()) { - const auto it = pt.find(key); - if (it != pt.not_found()) { - haveCXSmilesFields = true; - if (it->second.get_value()) { - cxSmilesFieldsFromJson |= - SmilesWrite::CXSmilesFields::_from_string(key)._to_integral(); - } - } - } + auto cxSmilesFieldsFromJson = + flagsFromJson(pt, &haveCXSmilesFields); if (haveCXSmilesFields) { cxSmilesFields = cxSmilesFieldsFromJson; } diff --git a/Code/GraphMol/SmilesParse/cxsmiles_test.cpp b/Code/GraphMol/SmilesParse/cxsmiles_test.cpp index a490814d8..e605c1f86 100644 --- a/Code/GraphMol/SmilesParse/cxsmiles_test.cpp +++ b/Code/GraphMol/SmilesParse/cxsmiles_test.cpp @@ -18,6 +18,7 @@ #include #include #include +#include #include "SmilesParse.h" #include "SmilesWrite.h" #include "SmartsWrite.h" @@ -1659,4 +1660,64 @@ TEST_CASE("Github #8586: MolFromSmiles loses atom maps if cxsmiles is used") { dlabel)); CHECK(dlabel == "d"); } -} \ No newline at end of file +} + +TEST_CASE("Test CXSmilesFields option parsing from JSON") { + SECTION("Empty JSON string preserves current values") { + std::uint32_t cxSmilesFields = + SmilesWrite::CXSmilesFields::CX_ALL_BUT_COORDS; + unsigned int restoreBondDirs = RestoreBondDirOptionClear; + updateCXSmilesFieldsFromJSON(cxSmilesFields, restoreBondDirs, "{}"); + CHECK(cxSmilesFields == SmilesWrite::CXSmilesFields::CX_ALL_BUT_COORDS); + CHECK(restoreBondDirs == RestoreBondDirOptionClear); + } + SECTION("No CXSmilesFields key preserves current values") { + std::uint32_t cxSmilesFields = + SmilesWrite::CXSmilesFields::CX_ALL_BUT_COORDS; + unsigned int restoreBondDirs = RestoreBondDirOptionTrue; + updateCXSmilesFieldsFromJSON( + cxSmilesFields, restoreBondDirs, + "{\"restoreBondDirOption\":\"RestoreBondDirOptionClear\"}"); + CHECK(cxSmilesFields == SmilesWrite::CXSmilesFields::CX_ALL_BUT_COORDS); + CHECK(restoreBondDirs == RestoreBondDirOptionClear); + } + SECTION("Multiple CXSmilesFields keys with true value are ORed together") { + std::uint32_t cxSmilesFields = SmilesWrite::CXSmilesFields::CX_ALL; + unsigned int restoreBondDirs = RestoreBondDirOptionClear; + updateCXSmilesFieldsFromJSON( + cxSmilesFields, restoreBondDirs, + "{\"CX_MOLFILE_VALUES\":true,\"CX_COORDS\":true}"); + CHECK(cxSmilesFields == (SmilesWrite::CXSmilesFields::CX_MOLFILE_VALUES | + SmilesWrite::CXSmilesFields::CX_COORDS)); + CHECK(restoreBondDirs == RestoreBondDirOptionClear); + } + SECTION( + "Multiple CXSmilesFields keys with true value are ORed together; adding unrelated false values makes no difference") { + std::uint32_t cxSmilesFields = SmilesWrite::CXSmilesFields::CX_ALL; + unsigned int restoreBondDirs = RestoreBondDirOptionClear; + updateCXSmilesFieldsFromJSON( + cxSmilesFields, restoreBondDirs, + "{\"CX_MOLFILE_VALUES\":true,\"CX_COORDS\":true,\"CX_ATOM_PROPS\":false,\"CX_BOND_CFG\":false}"); + CHECK(cxSmilesFields == (SmilesWrite::CXSmilesFields::CX_MOLFILE_VALUES | + SmilesWrite::CXSmilesFields::CX_COORDS)); + CHECK(restoreBondDirs == RestoreBondDirOptionClear); + } + SECTION( + "Multiple CXSmilesFields keys with true value are ORed together, then AND NOTed with ORed false values") { + std::uint32_t cxSmilesFields = SmilesWrite::CXSmilesFields::CX_ALL; + unsigned int restoreBondDirs = RestoreBondDirOptionClear; + updateCXSmilesFieldsFromJSON(cxSmilesFields, restoreBondDirs, + "{\"CX_ALL\":true,\"CX_COORDS\":false}"); + CHECK(cxSmilesFields == SmilesWrite::CXSmilesFields::CX_ALL_BUT_COORDS); + CHECK(restoreBondDirs == RestoreBondDirOptionClear); + } + SECTION( + "Multiple CXSmilesFields keys with true value are ORed together, then AND NOTed with ORed false values; order does not matter") { + std::uint32_t cxSmilesFields = SmilesWrite::CXSmilesFields::CX_ALL; + unsigned int restoreBondDirs = RestoreBondDirOptionClear; + updateCXSmilesFieldsFromJSON(cxSmilesFields, restoreBondDirs, + "{\"CX_COORDS\":false,\"CX_ALL\":true}"); + CHECK(cxSmilesFields == SmilesWrite::CXSmilesFields::CX_ALL_BUT_COORDS); + CHECK(restoreBondDirs == RestoreBondDirOptionClear); + } +} diff --git a/Code/MinimalLib/JSONParsers.cpp b/Code/MinimalLib/JSONParsers.cpp index 4b310cc60..3e814e7a7 100644 --- a/Code/MinimalLib/JSONParsers.cpp +++ b/Code/MinimalLib/JSONParsers.cpp @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include @@ -30,16 +31,8 @@ void updatePropertyPickleOptionsFromJSON(unsigned int &propFlags, boost::property_tree::read_json(ss, pt); const auto nodeIt = pt.find("propertyFlags"); if (nodeIt != pt.not_found()) { - auto propertyFlagsFromJson = - (+PicklerOps::PropertyPickleOptions::NoProps)._to_integral(); - for (const auto *key : PicklerOps::PropertyPickleOptions::_names()) { - if (nodeIt->second.get(key, false)) { - propertyFlagsFromJson |= - PicklerOps::PropertyPickleOptions::_from_string(key) - ._to_integral(); - } - } - propFlags = propertyFlagsFromJson; + propFlags = + flagsFromJson(nodeIt->second); } } } @@ -51,15 +44,7 @@ void updateSanitizeFlagsFromJSON(unsigned int &sanitizeFlags, boost::property_tree::ptree pt; ss.str(details_json); boost::property_tree::read_json(ss, pt); - auto sanitizeFlagsFromJson = - (+MolOps::SanitizeFlags::SANITIZE_NONE)._to_integral(); - for (const auto *key : MolOps::SanitizeFlags::_names()) { - if (pt.get(key, false)) { - sanitizeFlagsFromJson |= - MolOps::SanitizeFlags::_from_string(key)._to_integral(); - } - } - sanitizeFlags = sanitizeFlagsFromJson; + sanitizeFlags = flagsFromJson(pt); } } diff --git a/Code/RDGeneral/JSONHelpers.h b/Code/RDGeneral/JSONHelpers.h new file mode 100644 index 000000000..c04443270 --- /dev/null +++ b/Code/RDGeneral/JSONHelpers.h @@ -0,0 +1,36 @@ +#include +#include +#include +#include +#include + +// Convert a JSON object with boolean keys to a bitwise enum flag set. +// The T enum must have been declared as a BETTER_ENUM. +// The JSON object should have keys corresponding to the enum names, and boolean +// values. Example JSON: { "FlagA": true, "FlagB": false, "FlagC": true, +// "FlagD": false } This would set FlagA and FlagC in the resulting flag set, +// then clear FlagB and FlagD. If keysFound is provided and non-null, it will be +// set to true if any of the enum keys were found in the JSON object, false +// otherwise. +template +typename T::_integral flagsFromJson(const boost::property_tree::ptree &pt, + bool *keysFound = nullptr) { + std::array flagsByType; + flagsByType.fill(static_cast(0)); + if (keysFound) { + *keysFound = false; + } + for (const auto *key : T::_names()) { + const auto it = pt.find(key); + if (it == pt.not_found()) { + continue; + } + if (keysFound) { + *keysFound = true; + } + auto value = T::_from_string(key)._to_integral(); + auto i = static_cast(it->second.template get_value()); + flagsByType[i] |= value; + } + return (flagsByType[1] & ~flagsByType[0]); +}