Merge branch 'develop' of github.com:pdb-redo/libcifpp into develop

CMakeLists.txt

@@ -27,7 +27,7 @@ cmake_minimum_required(VERSION 3.16)
 # set the project name
 project(
 	libcifpp
-	VERSION 7.0.1
+	VERSION 7.0.2
 	LANGUAGES CXX)
 
 list(PREPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake")

changelog

+Version 7.0.2
+- Fix in testing error_code results.
+
 Version 7.0.1
 - Various reconstruction fixes
 - category order in output fixed
+- better implementation of constructors for file, datablock and category
+- small optimisation in iterator
 
 Version 7.0.0
 - Renaming many methods and parameters to be more

include/cif++/category.hpp

@@ -182,7 +182,7 @@ class category
 
 	/// @brief Update the links in this category
 	/// @param db The enclosing @ref datablock
-	void update_links(datablock &db);
+	void update_links(const datablock &db);
 
 	/// @brief Return the global @ref validator for the data
 	/// @return The @ref validator or nullptr if not assigned
@@ -1263,6 +1263,7 @@ class category
 		{
 		}
 
+// TODO: NEED TO FIX THIS!
 		category *linked;
 		const link_validator *v;
 	};

include/cif++/item.hpp

@@ -117,7 +117,7 @@ class item
 		char buffer[32];
 
 		auto r = to_chars(buffer, buffer + sizeof(buffer) - 1, value, chars_format::fixed, precision);
-		if (r.ec != std::errc())
+		if ((bool)r.ec)
 			throw std::runtime_error("Could not format number");
 
 		m_value.assign(buffer, r.ptr - buffer);
@@ -136,7 +136,7 @@ class item
 		char buffer[32];
 
 		auto r = to_chars(buffer, buffer + sizeof(buffer) - 1, value, chars_format::general);
-		if (r.ec != std::errc())
+		if ((bool)r.ec)
 			throw std::runtime_error("Could not format number");
 
 		m_value.assign(buffer, r.ptr - buffer);
@@ -151,7 +151,7 @@ class item
 		char buffer[32];
 
 		auto r = std::to_chars(buffer, buffer + sizeof(buffer) - 1, value);
-		if (r.ec != std::errc())
+		if ((bool)r.ec)
 			throw std::runtime_error("Could not format number");
 
 		m_value.assign(buffer, r.ptr - buffer);
@@ -560,7 +560,7 @@ struct item_handle::item_value_as<T, std::enable_if_t<std::is_arithmetic_v<T> an
 
 			std::from_chars_result r = (b + 1 < e and *b == '+' and std::isdigit(b[1])) ? selected_charconv<value_type>::from_chars(b + 1, e, result) : selected_charconv<value_type>::from_chars(b, e, result);
 
-			if (r.ec != std::errc() or r.ptr != e)
+			if ((bool)r.ec or r.ptr != e)
 			{
 				result = {};
 				if (cif::VERBOSE)
@@ -595,7 +595,7 @@ struct item_handle::item_value_as<T, std::enable_if_t<std::is_arithmetic_v<T> an
 
 			std::from_chars_result r = (b + 1 < e and *b == '+' and std::isdigit(b[1])) ? selected_charconv<value_type>::from_chars(b + 1, e, v) : selected_charconv<value_type>::from_chars(b, e, v);
 
-			if (r.ec != std::errc() or r.ptr != e)
+			if ((bool)r.ec or r.ptr != e)
 			{
 				if (cif::VERBOSE)
 				{

include/cif++/model.hpp

@@ -741,7 +741,7 @@ class sugar : public residue
 	{
 		int result;
 		auto r = std::from_chars(m_auth_seq_id.data(), m_auth_seq_id.data() + m_auth_seq_id.length(), result);
-		if (r.ec != std::errc())
+		if ((bool)r.ec)
 			throw std::runtime_error("The auth_seq_id should be a number for a sugar");
 		return result;
 	}

include/cif++/text.hpp

@@ -383,7 +383,7 @@ std::from_chars_result from_chars(const char *first, const char *last, FloatType
 	int exponent = 0;
 	bool done = false;
 
-	while (not done and result.ec == std::errc())
+	while (not done and not (bool)result.ec)
 	{
 		char ch = result.ptr != last ? *result.ptr : 0;
 		++result.ptr;
@@ -467,7 +467,7 @@ std::from_chars_result from_chars(const char *first, const char *last, FloatType
 		}
 	}
 
-	if (result.ec == std::errc())
+	if (not (bool)result.ec)
 	{
 		long double v = f * vi * sign;
 		if (exponent != 0)

src/category.cpp

@@ -666,7 +666,7 @@ void category::set_validator(const validator *v, datablock &db)
 	update_links(db);
 }
 
-void category::update_links(datablock &db)
+void category::update_links(const datablock &db)
 {
 	m_child_links.clear();
 	m_parent_links.clear();
@@ -675,7 +675,7 @@ void category::update_links(datablock &db)
 	{
 		for (auto link : m_validator->get_links_for_parent(m_name))
 		{
-			auto childCat = db.get(link->m_child_category);
+			auto childCat = const_cast<category *>(db.get(link->m_child_category));
 			if (childCat == nullptr)
 				continue;
 			m_child_links.emplace_back(childCat, link);
@@ -683,7 +683,7 @@ void category::update_links(datablock &db)
 
 		for (auto link : m_validator->get_links_for_child(m_name))
 		{
-			auto parentCat = db.get(link->m_parent_category);
+			auto parentCat = const_cast<category *>(db.get(link->m_parent_category));
 			if (parentCat == nullptr)
 				continue;
 			m_parent_links.emplace_back(parentCat, link);
@@ -791,7 +791,7 @@ bool category::is_valid() const
 
 				iv->validate_value(vi->text(), ec);
 
-				if (ec != std::errc())
+				if ((bool)ec)
 				{
 					m_validator->report_error(ec, m_name, m_items[cix].m_name, false);
 					continue;

src/compound.cpp

@@ -550,12 +550,6 @@ class local_compound_factory_impl : public compound_factory_impl
 		for (const auto &[id, name, threeLetterCode, group] :
 			file["comp_list"]["chem_comp"].rows<std::string, std::string, std::string, std::string>("id", "name", "three_letter_code", "group"))
 		{
-			// if (std::regex_match(group, peptideRx))
-			// 	mKnownPeptides.insert(threeLetterCode);
-			// else if (cif::iequals(group, "DNA") or cif::iequals(group, "RNA"))
-			// 	mKnownBases.insert(threeLetterCode);
-			
-			// Test if this compound is known in CCD
 			auto &rdb = m_local_file["comp_" + id];
 			if (rdb.empty())
 			{
@@ -563,6 +557,53 @@ class local_compound_factory_impl : public compound_factory_impl
 				continue;
 			}
 
+			construct_compound(rdb, id, name, threeLetterCode, group);
+		}
+	}
+
+	compound *create(const std::string &id) override;
+
+  private:
+
+	compound *construct_compound(const datablock &db, const std::string &id, const std::string &name, const std::string &three_letter_code, const std::string &group);
+
+	cif::file m_local_file;
+};
+
+compound *local_compound_factory_impl::create(const std::string &id)
+{
+	compound *result = nullptr;
+
+	for (auto &db : m_local_file)
+	{
+		if (db.name() == "comp_" + id)
+		{
+			auto chem_comp = db.get("chem_comp");
+			if (not chem_comp)
+				break;
+
+			try
+			{
+				const auto &[id, name, threeLetterCode, group] =
+					chem_comp->front().get<std::string, std::string, std::string, std::string>("id", "name", "three_letter_code", "group");
+
+				result = construct_compound(db, id, name, threeLetterCode, group);
+			}
+			catch (const std::exception &ex)
+			{
+				std::throw_with_nested(std::runtime_error("Error loading compound " + id));
+			}
+
+			break;
+		}
+	}
+
+	return result;
+}
+
+compound *local_compound_factory_impl::construct_compound(const datablock &rdb, const std::string &id,
+	const std::string &name, const std::string &three_letter_code, const std::string &group)
+{
 	cif::datablock db(id);
 
 	float formula_weight = 0;
@@ -644,48 +685,15 @@ class local_compound_factory_impl : public compound_factory_impl
 		{ "formula", formula },
 		{ "pdbx_formal_charge", formal_charge },
 		{ "formula_weight", formula_weight },
-				{ "three_letter_code", threeLetterCode }
+		{ "three_letter_code", three_letter_code }
 	});
 
-			m_compounds.push_back(new compound(db));
-		}
-	}
-
-	// compound *create(const std::string &id) override;
-
-  private:
-	cif::file m_local_file;
-};
-
-// compound *local_compound_factory_impl::create(const std::string &id)
-// {
-// 	compound *result = nullptr;
-
-// 	for (auto &db : m_local_file)
-// 	{
-// 		if (db.name() == id)
-// 		{
-// 			cif::datablock db_copy(db);
-
-// 			try
-// 			{
-// 				result = new compound(db_copy, 1);
-// 			}
-// 			catch (const std::exception &ex)
-// 			{
-// 				std::throw_with_nested(std::runtime_error("Error loading compound " + id));
-// 			}
-
-
-// 			std::shared_lock lock(mMutex);
-// 			m_compounds.push_back(result);
-
-// 			break;
-// 		}
-// 	}
+	std::shared_lock lock(mMutex);
 
-// 	return result;
-// }
+	auto result = new compound(db);
+	m_compounds.push_back(result);
+	return result;
+}
 
 // --------------------------------------------------------------------
 

src/datablock.cpp

@@ -109,6 +109,9 @@ bool datablock::validate_links() const
 	bool result = true;
 
 	for (auto &cat : *this)
+		const_cast<category &>(cat).update_links(*this);
+
+	for (auto &cat : *this)
 		result = cat.validate_links() and result;
 
 	return result;
@@ -175,6 +178,11 @@ std::tuple<datablock::iterator, bool> datablock::emplace(std::string_view name)
 	}
 
 	assert(i != end());
+
+	// links may have changed...
+	for (auto &cat : *this)
+		cat.update_links(*this);
+
 	return std::make_tuple(i, is_new);
 }
 

src/model.cpp

@@ -74,7 +74,7 @@ int atom::atom_impl::get_property_int(std::string_view name) const
 		auto s = get_property(name);
 
 		std::from_chars_result r = std::from_chars(s.data(), s.data() + s.length(), result);
-		if (r.ec != std::errc() and VERBOSE > 0)
+		if ((bool)r.ec and VERBOSE > 0)
 			std::cerr << "Error converting " << s << " to number for property " << name << '\n';
 	}
 	return result;
@@ -88,7 +88,7 @@ float atom::atom_impl::get_property_float(std::string_view name) const
 		auto s = get_property(name);
 
 		std::from_chars_result r = cif::from_chars(s.data(), s.data() + s.length(), result);
-		if (r.ec != std::errc() and VERBOSE > 0)
+		if ((bool)r.ec and VERBOSE > 0)
 			std::cerr << "Error converting " << s << " to number for property " << name << '\n';
 	}
 	return result;

src/pdb/cif2pdb.cpp

@@ -681,7 +681,7 @@ class Fi : public FBase
 		{
 			long l = 0;
 			auto r = std::from_chars(s.data(), s.data() + s.length(), l);
-			if (r.ec != std::errc())
+			if ((bool)r.ec)
 			{
 				if (VERBOSE > 0)
 					std::cerr << "Failed to write '" << s << "' as a long from field " << mField << ", this indicates an error in the code for writing PDB files\n";
@@ -719,7 +719,7 @@ class Ff : public FBase
 
 			double d = 0;
 			auto r = cif::from_chars(s.data(), s.data() + s.length(), d);
-			if (r.ec != std::errc())
+			if ((bool)r.ec)
 			{
 				if (VERBOSE > 0)
 					std::cerr << "Failed to write '" << s << "' as a double from field " << mField << ", this indicates an error in the code for writing PDB files\n";
@@ -3393,7 +3393,7 @@ std::tuple<int, int> WriteCoordinatesForModel(std::ostream &pdbFile, const datab
 		{
 			int nr = 0;
 			auto r = std::from_chars(modelNum.data(), modelNum.data() + modelNum.length(), nr);
-			if (r.ec != std::errc())
+			if ((bool)r.ec)
 			{
 				if (VERBOSE > 0)
 					std::cerr << "Model number '" << modelNum << "' is not a valid integer\n";

src/pdb/pdb2cif.cpp

@@ -1132,7 +1132,7 @@ void PDBFileParser::PreParseInput(std::istream &is)
 		if (not cs.empty())
 		{
 			auto r = std::from_chars(cs.data(), cs.data() + cs.length(), result);
-			if (r.ec != std::errc())
+			if ((bool)r.ec)
 				throw std::runtime_error("Continuation std::string '" + cs + "' is not valid");
 		}
 
@@ -1397,7 +1397,7 @@ void PDBFileParser::PreParseInput(std::istream &is)
 			{
 				auto f = cur->vF(74, 78);
 				auto r = cif::from_chars(f.data(), f.data() + f.length(), link.distance);
-				if (r.ec != std::errc() and cif::VERBOSE > 0)
+				if ((bool)r.ec and cif::VERBOSE > 0)
 					std::cerr << "Error parsing link distance at line " << cur->mLineNr << '\n';
 			}
 			//	74 – 78         Real(5.2)      Length          Link distance
@@ -5306,7 +5306,7 @@ void PDBFileParser::ParseConnectivtyAnnotation()
 
 				double d;
 				auto r = cif::from_chars(distance.data(), distance.data() + distance.length(), d);
-				if (r.ec != std::errc())
+				if ((bool)r.ec)
 				{
 					if (cif::VERBOSE > 0)
 						std::cerr << "Distance value '" << distance << "' is not a valid float in LINK record\n";

src/pdb/reconstruct.cpp

@@ -519,14 +519,14 @@ void checkAtomRecords(datablock &db)
 
 			float v;
 			auto s = row.get<std::string>(item_name);
-			if (auto [ptr, ec] = cif::from_chars(s.data(), s.data() + s.length(), v); ec != std::errc())
+			if (auto [ptr, ec] = cif::from_chars(s.data(), s.data() + s.length(), v); (bool)ec)
 				continue;
 
 			if (s.length() < prec + 1 or s[s.length() - prec - 1] != '.')
 			{
 				char b[12];
 
-				if (auto [ptr, ec] = cif::to_chars(b, b + sizeof(b), v, cif::chars_format::fixed, prec); ec == std::errc())
+				if (auto [ptr, ec] = cif::to_chars(b, b + sizeof(b), v, cif::chars_format::fixed, prec); (bool)ec)
 					row.assign(item_name, { b, static_cast<std::string::size_type>(ptr - b) }, false, false);
 			}
 		}

src/pdb/validate-pdbx.cpp

@@ -71,7 +71,7 @@ bool is_valid_pdbx_file(const file &file, std::string_view dictionary)
 {
 	std::error_code ec;
 	bool result = is_valid_pdbx_file(file, dictionary, ec);
-	return result and ec == std::errc();
+	return result and not (bool)ec;
 }
 
 bool is_valid_pdbx_file(const file &file, std::error_code &ec)
@@ -326,7 +326,7 @@ bool is_valid_pdbx_file(const file &file, std::string_view dictionary, std::erro
 		ec = make_error_code(validation_error::not_valid_pdbx);
 	}
 
-	if (not result and ec == std::errc())
+	if (not result and (bool)ec)
 		ec = make_error_code(validation_error::not_valid_pdbx);
 
 	return result;

src/symmetry.cpp

@@ -111,7 +111,7 @@ sym_op::sym_op(std::string_view s)
 	m_tb = r.ptr[2] - '0';
 	m_tc = r.ptr[3] - '0';
 
-	if (r.ec != std::errc() or rnri > 192 or r.ptr[0] != '_' or m_ta > 9 or m_tb > 9 or m_tc > 9)
+	if ((bool)r.ec or rnri > 192 or r.ptr[0] != '_' or m_ta > 9 or m_tb > 9 or m_tc > 9)
 		throw std::invalid_argument("Could not convert string into sym_op");
 }
 
@@ -119,7 +119,7 @@ std::string sym_op::string() const
 {
 	char b[9];
 	auto r = std::to_chars(b, b + sizeof(b), m_nr);
-	if (r.ec != std::errc() or r.ptr > b + 4)
+	if ((bool)r.ec or r.ptr > b + 4)
 		throw std::runtime_error("Could not write out symmetry operation to string");
 	
 	*r.ptr++ = '_';

src/symop-map-generator.cpp

@@ -280,7 +280,7 @@ int main(int argc, char* const argv[])
 			if (std::isdigit(line[0]))	// start of new spacegroup
 			{
 				auto r = std::from_chars(line.data(), line.data() + line.length(), sgnr);
-				if (r.ec != std::errc())
+				if ((bool)r.ec)
 					throw std::runtime_error("Error parsing symop.lib file");
 				rnr = 1;
 				continue;

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 (ra.ec == std::errc() and rb.ec == std::errc())
+				if (not (bool)ra.ec and not (bool)rb.ec)
 				{
 					auto d = da - db;
 					if (std::abs(d) > std::numeric_limits<double>::epsilon())
@@ -149,7 +149,7 @@ int type_validator::compare(std::string_view a, std::string_view b) const
 							result = -1;
 					}
 				}
-				else if (ra.ec == std::errc())
+				else if ((bool)ra.ec)
 					result = 1;
 				else
 					result = -1;
@@ -222,7 +222,7 @@ void item_validator::operator()(std::string_view value) const
 
 bool item_validator::validate_value(std::string_view value, std::error_code &ec) const noexcept
 {
-	ec = {};
+	ec.clear();
 
 	if (not value.empty() and value != "?" and value != ".")
 	{
@@ -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 ec == std::errc();
+	return not (bool)ec;
 }
 
 // --------------------------------------------------------------------

test/reconstruction-test.cpp

@@ -54,7 +54,7 @@ TEST_CASE("reconstruct")
 
 			std::error_code ec;
 			CHECK_FALSE(cif::pdb::is_valid_pdbx_file(f, ec));
-			CHECK(ec != std::errc{});
+			CHECK((bool)ec);
 
 			CHECK(cif::pdb::reconstruct_pdbx(f));
 		}

test/unit-v2-test.cpp

@@ -101,7 +101,7 @@ TEST_CASE("cc_1")
 		float tv;
 		const auto &[ptr, ec] = cif::from_chars(txt.data(), txt.data() + txt.length(), tv);
 
-		REQUIRE(ec == std::errc());
+		CHECK_FALSE((bool)ec);
 		REQUIRE(tv == val);
 		if (ch != 0)
 			REQUIRE(*ptr == ch);
@@ -119,7 +119,7 @@ TEST_CASE("cc_2")
 		char buffer[64];
 		const auto &[ptr, ec] = cif::to_chars(buffer, buffer + sizeof(buffer), val, cif::chars_format::fixed, prec);
 
-		REQUIRE(ec == std::errc());
+		CHECK_FALSE((bool)ec);
 
 		REQUIRE(buffer == test);
 	}