Fixes for dictionary loading

CMakeLists.txt

@@ -27,7 +27,7 @@ cmake_minimum_required(VERSION 3.23)
 # set the project name
 project(
 	libcifpp
-	VERSION 7.0.8
+	VERSION 7.0.9
 	LANGUAGES CXX)
 
 list(PREPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake")
@@ -42,6 +42,11 @@ include(GenerateExportHeader)
 include(CTest)
 include(FetchContent)
 
+# FindBoost, take care of it now.
+if(CMAKE_VERSION VERSION_GREATER_EQUAL 3.30)
+	cmake_policy(SET CMP0167 NEW)
+endif()
+
 # When building with ninja-multiconfig, build both debug and release by default
 if(CMAKE_GENERATOR STREQUAL "Ninja Multi-Config")
 	set(CMAKE_CROSS_CONFIGS "Debug;Release")
@@ -534,7 +539,7 @@ write_basic_package_version_file(
 	VERSION ${PROJECT_VERSION}
 	COMPATIBILITY AnyNewerVersion)
 
-if(BUILD_TESTING)
+if(BUILD_TESTING AND PROJECT_IS_TOP_LEVEL)
 	add_subdirectory(test)
 endif()
 

changelog

@@ -1,3 +1,11 @@
+Version 7.0.9
+- Using cif::file::load_dictionary it is now possible to
+  load a dictionary along with its extensions in one go.
+  E.g. file.load_dictionary("mmcif_pdbx;dssp-extension")
+- Fix in compound factory to avoid errors with lower case
+  compound id's
+- Fix sac_parser's index to be case insensitive
+
 Version 7.0.8
 - Fix PDB Remark 3 parser
 - Added three way comparison for point

include/cif++/compound.hpp

@@ -180,8 +180,7 @@ class compound
 	friend class local_compound_factory_impl;
 
 	compound(cif::datablock &db);
-	compound(cif::datablock &db, int);
-
+	
 	std::string m_id;
 	std::string m_name;
 	std::string m_type;

include/cif++/file.hpp

@@ -176,6 +176,17 @@ class file : public std::list<datablock>
 	 * @brief Attempt to load the named dictionary @a name and
 	 * create a validator based on it.
 	 * 
+	 * Tje @a name can be the name of a single file, or even the
+	 * stem of that filename. So, e.g. mmcif_pdbx is valid.
+	 * 
+	 * Since libcifpp can use extensions to validators, you
+	 * can add them to the name. So if you would like to add
+	 * the dssp extensions you would have to write:
+	 * 
+	 * @code{cpp}
+	 * file.load_dictionary("mmcif_pdbx;dssp-extension");
+	 * @endcode
+	 * 
 	 * @param name The name of the dictionary to load
 	 */
 	void load_dictionary(std::string_view name);

include/cif++/parser.hpp

@@ -73,7 +73,17 @@ class sac_parser
 {
   public:
 	/** @cond */
-	using datablock_index = std::map<std::string, std::size_t>;
+	struct iless_op
+	{
+		bool operator()(std::string a, std::string b) const
+		{
+			to_upper(a);
+			to_upper(b);
+			return a < b;
+		}
+	};
+
+	using datablock_index = std::map<std::string, std::size_t, iless_op>;
 
 	virtual ~sac_parser() = default;
 	/** @endcond */

src/compound.cpp

@@ -177,89 +177,6 @@ compound::compound(cif::datablock &db)
 	}
 }
 
-compound::compound(cif::datablock &db, int)
-{
-	auto &chemComp = db["chem_comp"];
-
-	if (chemComp.size() != 1)
-		throw std::runtime_error("Invalid compound file, chem_comp should contain a single row");
-
-	cif::tie(m_id, m_name) =
-		chemComp.front().get("id", "name");
-	
-	cif::trim(m_name);
-
-	m_type = "NON-POLYMER";
-
-	auto &chemCompAtom = db["chem_comp_atom"];
-	for (auto row : chemCompAtom)
-	{
-		compound_atom atom;
-		std::string type_symbol;
-		cif::tie(atom.id, type_symbol, atom.charge, atom.x, atom.y, atom.z) =
-			row.get("atom_id", "type_symbol", "charge", "x", "y", "z");
-		atom.type_symbol = atom_type_traits(type_symbol).type();
-
-		m_formal_charge += atom.charge;
-
-		m_atoms.push_back(std::move(atom));
-	}
-
-	auto &chemCompBond = db["chem_comp_bond"];
-	for (auto row : chemCompBond)
-	{
-		compound_bond bond;
-		std::string btype;
-		cif::tie(bond.atom_id[0], bond.atom_id[1], btype, bond.aromatic) = row.get("atom_id_1", "atom_id_2", "type", "aromatic");
-
-		using cif::iequals;
-
-		if (iequals(btype, "single"))
-			bond.type = bond_type::sing;
-		else if (iequals(btype, "double"))
-			bond.type = bond_type::doub;
-		else if (iequals(btype, "triple"))
-			bond.type = bond_type::trip;
-		else if (iequals(btype, "deloc") or iequals(btype, "aromat") or iequals(btype, "aromatic"))
-			bond.type = bond_type::delo;
-		else
-		{
-			if (cif::VERBOSE > 0)
-				std::cerr << "Unimplemented chem_comp_bond.type " << btype << " in " << db.name() << '\n';
-			bond.type = bond_type::sing;
-		}
-		m_bonds.push_back(std::move(bond));
-	}
-
-	// reconstruct a formula and weight
-
-	m_formula_weight = 0;
-
-	std::map<atom_type, int> f;
-	for (auto &atom : m_atoms)
-		f[atom.type_symbol] += 1;
-	
-	if (f.count(atom_type::C))
-	{
-		atom_type_traits att(atom_type::C);
-		m_formula += att.symbol() + std::to_string(f[atom_type::C]) + ' ';
-		m_formula_weight += att.weight() * f[atom_type::C];
-	}
-
-	for (const auto &[type, count] : f)
-	{
-		if (type == atom_type::C)
-			continue;
-
-		atom_type_traits att(type);
-		m_formula += att.symbol() + std::to_string(count) + ' ';
-		m_formula_weight += att.weight() * count;
-	}
-	
-	if (not m_formula.empty())
-		m_formula.pop_back();
-}
-
 compound_atom compound::get_atom_by_atom_id(const std::string &atom_id) const
 {
 	compound_atom result = {};
@@ -379,8 +296,6 @@ class compound_factory_impl : public std::enable_shared_from_this<compound_facto
 
 	compound *get(std::string id)
 	{
-		cif::to_upper(id);
-
 		std::shared_lock lock(mMutex);
 
 		compound *result = nullptr;
@@ -401,7 +316,8 @@ class compound_factory_impl : public std::enable_shared_from_this<compound_facto
 				break;
 		}
 
-		if (result == nullptr and m_missing.count(id) == 0)
+		if (result == nullptr and
+			find_if(m_missing.begin(), m_missing.end(), [&id](const std::string &m_id) { return cif::iequals(id, m_id); }) == m_missing.end())
 		{
 			for (auto impl = shared_from_this(); impl; impl = impl->m_next)
 			{
@@ -411,7 +327,7 @@ class compound_factory_impl : public std::enable_shared_from_this<compound_facto
 			}
 
 			if (result == nullptr)
-				m_missing.insert(id);
+				m_missing.emplace_back(id);
 		}
 
 		return result;
@@ -444,7 +360,7 @@ class compound_factory_impl : public std::enable_shared_from_this<compound_facto
 	cif::parser::datablock_index m_index;
 
 	std::vector<compound *> m_compounds;
-	std::set<std::string> m_missing;
+	std::vector<std::string> m_missing;
 	std::shared_ptr<compound_factory_impl> m_next;
 };
 
@@ -576,7 +492,7 @@ compound *local_compound_factory_impl::create(const std::string &id)
 
 	for (auto &db : m_local_file)
 	{
-		if (db.name() == "comp_" + id)
+		if (db.name() == id)
 		{
 			auto chem_comp = db.get("chem_comp");
 			if (not chem_comp)

src/validate.cpp

@@ -138,7 +138,7 @@ int type_validator::compare(std::string_view a, std::string_view b) const
 				ra = selected_charconv<double>::from_chars(a.data(), a.data() + a.length(), da);
 				rb = selected_charconv<double>::from_chars(b.data(), b.data() + b.length(), db);
 
-				if (not (bool)ra.ec and not (bool)rb.ec)
+				if (not(bool) ra.ec and not(bool) rb.ec)
 				{
 					auto d = da - db;
 					if (std::abs(d) > std::numeric_limits<double>::epsilon())
@@ -232,7 +232,7 @@ bool item_validator::validate_value(std::string_view value, std::error_code &ec)
 			ec = make_error_code(validation_error::value_is_not_in_enumeration_list);
 	}
 
-	return not (bool)ec;
+	return not(bool) ec;
 }
 
 // --------------------------------------------------------------------
@@ -408,9 +408,7 @@ void validator::report_error(std::error_code ec, bool fatal) const
 void validator::report_error(std::error_code ec, std::string_view category,
 	std::string_view item, bool fatal) const
 {
-	auto ex = item.empty() ?
-		validation_exception(ec, category) :
-		validation_exception(ec, category, item);
+	auto ex = item.empty() ? validation_exception(ec, category) : validation_exception(ec, category, item);
 
 	if (m_strict or fatal)
 		throw ex;
@@ -455,59 +453,71 @@ const validator &validator_factory::operator[](std::string_view dictionary_name)
 		}
 
 		// not found, add it
-		auto data = load_resource(dictionary_name);
 
-		if (not data and dictionary.extension().string() != ".dic")
-			data = load_resource(dictionary.parent_path() / (dictionary.filename().string() + ".dic"));
+		validator v(dictionary_name);
 
-		if (data)
-			construct_validator(dictionary_name, *data);
-		else
+		for (bool first = true; auto part_name : cif::split(dictionary_name, ";", true))
 		{
-			std::error_code ec;
+			auto data = load_resource(part_name);
+			dictionary.assign(part_name.begin(), part_name.end());
 
-			// might be a compressed dictionary on disk
-			std::filesystem::path p = dictionary;
-			if (p.extension() == ".dic")
-				p = p.parent_path() / (p.filename().string() + ".gz");
-			else
-				p = p.parent_path() / (p.filename().string() + ".dic.gz");
+			if (not data and dictionary.extension().string() != ".dic")
+				data = load_resource(dictionary.parent_path() / (dictionary.filename().string() + ".dic"));
+
+			if (not data)
+			{
+				std::error_code ec;
+
+				// might be a compressed dictionary on disk
+				std::filesystem::path p = dictionary;
+				if (p.extension() == ".dic")
+					p = p.parent_path() / (p.filename().string() + ".gz");
+				else
+					p = p.parent_path() / (p.filename().string() + ".dic.gz");
 
 #if defined(CACHE_DIR) or defined(DATA_DIR)
-			if (not std::filesystem::exists(p, ec) or ec)
-			{
-				for (const char *dir : {
+				if (not std::filesystem::exists(p, ec) or ec)
+				{
+					for (const char *dir : {
 # if defined(CACHE_DIR)
-						 CACHE_DIR,
+							 CACHE_DIR,
 # endif
 # if defined(DATA_DIR)
 							 DATA_DIR
 # endif
-					 })
-				{
-					auto p2 = std::filesystem::path(dir) / p;
-					if (std::filesystem::exists(p2, ec) and not ec)
+						 })
 					{
-						swap(p, p2);
-						break;
+						auto p2 = std::filesystem::path(dir) / p;
+						if (std::filesystem::exists(p2, ec) and not ec)
+						{
+							swap(p, p2);
+							break;
+						}
 					}
 				}
-			}
 #endif
 
-			if (std::filesystem::exists(p, ec) and not ec)
-			{
-				gzio::ifstream in(p);
+				if (std::filesystem::exists(p, ec) and not ec)
+				{
+					auto in = std::make_unique<gzio::ifstream>(p);
 
-				if (not in.is_open())
-					throw std::runtime_error("Could not open dictionary (" + p.string() + ")");
+					if (not in->is_open())
+						throw std::runtime_error("Could not open dictionary (" + p.string() + ")");
 
-				construct_validator(dictionary_name, in);
+					data.reset(in.release());
+				}
+				else
+					throw std::runtime_error("Dictionary not found or defined (" + dictionary.string() + ")");
 			}
+
+			if (std::exchange(first, false))
+				v = parse_dictionary(part_name, *data);
 			else
-				throw std::runtime_error("Dictionary not found or defined (" + dictionary.string() + ")");
+				extend_dictionary(v, *data);
 		}
 
+		m_validators.emplace_back(std::move(v));
+
 		return m_validators.back();
 	}
 	catch (const std::exception &ex)

test/unit-v2-test.cpp

@@ -3482,7 +3482,7 @@ TEST_CASE("compound_test_1")
 	cif::compound_factory::instance().push_dictionary(gTestDir / "REA_v2.cif");
 	auto compound = cif::compound_factory::instance().create("REA_v2");
 	REQUIRE(compound != nullptr);
-	REQUIRE(compound->id() == "REA_v2");
+	REQUIRE(cif::iequals(compound->id(), "REA_v2"));
 }
 
 // --------------------------------------------------------------------